-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move D3D11 API usage out of dxbc_debug.cpp #1457
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in principle this change is OK but the split of abstraction needs to be moved for the texture operations. The maths operations are nicely abstracted so there's no real dependency there - it takes an input and gives an output - and I'd like to see the texture operations organised the same way. That way rather than this being a move of code from one place to another, it's a better separation of concerns.
I haven't gone into finest detail over the rest of the code since it's a) largely identical and just moved and b) may change given the above so I'd be wasting your time. I'll re-review in more detail once the changes above are made.
I left a couple of notes about final tidying up but otherwise this looks good to me now. |
case 2: channel = "Blue"; break; | ||
case 3: channel = "Alpha"; break; | ||
} | ||
gatherChannel = static_cast<GatherChannel>(op.operands[3].comps[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer a normal cast, not a C++ cast here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one minor warning in the CI to fix and a style change, otherwise this LGTM.
1671879
to
bc27be2
Compare
if(retFmt == DXGI_FORMAT_UNKNOWN) | ||
{ | ||
funcRet = buf; | ||
retFmt = fmts[resourceData.dim]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a typo here while testing - this should be indexed by resourceData.retType
not resourceData.dim
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, thanks for catching. Fixed.
The following instructions used D3D11 calls to compute results, and have been moved to an API wrapper class that is defined in d3d11_shaderdebug.cpp: transcendentals (rcp, rsqrt, exp2, log2, and sincos), sample info/pos, bufinfo, resinfo, and sample/load/gather/lod.
The following instructions used D3D11 calls to compute results, and have been
moved to an API wrapper class that is defined in d3d11_shaderdebug.cpp:
transcendentals (rcp, rsqrt, exp2, log2, and sincos), sample info/pos, bufinfo,
resinfo, and sample/load/gather/lod.