-
Notifications
You must be signed in to change notification settings - Fork 884
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
[d3d9] Track dirty regions for UpdateTexture #1759
Conversation
4736209
to
d8f38ae
Compare
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.
Also needs to handle:
- Autogen mipmaps (should probably dirty everything?)
- Render target/depth stencil
- pInitialData
Does native actually hold a list of boxes to update? Or does it just grow a single box?
pInitialData is handled (notice I set the whole texture dirty in the constructor of CommonTexture). Aside from that, I've implemented it the way the docs describe it:
Why does auto mip gen matter? We only track mip 0.
Honestly no idea. Nine just grows a single box. I initially did multiple boxes and have changed it to a single box since. That seems good enough for the broken game. |
I am mainly thinking of the case where someone calls UpdateTexture after mips have been generated. |
|
Adjusted how auto mip gen textures are handled in UpdateTexture according to the docs above. |
src/d3d9/d3d9_device.cpp
Outdated
VkDeviceSize srcOffset = srcByteOffset.y * formatInfo->elementSize * blockCount.width | ||
+ srcByteOffset.x * formatInfo->elementSize; | ||
VkExtent2D srcExtent = VkExtent2D{ blockCount.width * formatInfo->blockSize.width, | ||
blockCount.height * formatInfo->blockSize.height }; |
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.
spacing
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.
Do we need anything more for srcOffset on 3D images?
src/d3d9/d3d9_device.cpp
Outdated
offset.x / int32_t(formatInfo->blockSize.width), | ||
offset.y / int32_t(formatInfo->blockSize.height) | ||
}; | ||
VkDeviceSize srcOffset = srcByteOffset.y * formatInfo->elementSize * blockCount.width |
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.
Do we need to do anything different here for 3D textures?
uint32_t mipLevels = std::min(srcTexInfo->Desc()->MipLevels, dstTexInfo->Desc()->MipLevels); | ||
uint32_t arraySlices = std::min(srcTexInfo->Desc()->ArraySize, dstTexInfo->Desc()->ArraySize); | ||
|
||
if (unlikely(srcTexInfo->IsAutomaticMip() && !dstTexInfo->IsAutomaticMip())) |
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'm scared of this
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.
Docs seem to say this is correct so I guess we can go with it for now...
Docs have lied in the past however 🐸
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.
Write a test then.
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.
FWIW, Nine does the same thing.
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/gallium/frontends/nine/device9.c#L1460
81d5eed
to
31c0edb
Compare
31c0edb
to
d640a76
Compare
Going to merge this so it can get some testing. |
Fixes #1757
Battle Engine Aquila relies on us not copying anything outside of the dirty regions that were set by
LockRect
andAddDirtyRect
.This could technically either be considered a game bug if the documentation is to be believed:
This has serious potential for regressions and I have not yet tested it with a variety of games. I'd like to get some feedback on the implementation first.