Fix Windows external texture interference #25193
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
lgtm for the unbind cleanup here.
Will follow up to see if I can find the location where the texture was being accidentally used in the icon font rendering -- seems like we should be ensuring no previously bound texture is being used accidentally there.
This feels really similar to the multitexturing font bugs we used to see on iOS.
/cc @jvanverth who I recall I'd discussed that issue with years ago for any thoughts/insights.
This will cause the previous texture binding used by Skia to be
incorrectly unbound, such that should they need to re-used an existing
binding, we'll end up rendering blank instead.
Instead we should be using
context->flushAndSubmit();
context->resetContext(kAll_GrBackendState);
in `EmbedderExternalTextureGL::ResolveTexture` in order to notify Skia
that we've updated handles within the current binding and invalidate any
assumptions about previous modifications it has made to the context.
This reverts commit d6e5946.
…lutter#25292) This will cause the previous texture binding used by Skia to be incorrectly unbound, such that should they need to re-used an existing binding, we'll end up rendering blank instead. Instead we should be using context->flushAndSubmit(); context->resetContext(kAll_GrBackendState); in `EmbedderExternalTextureGL::ResolveTexture` in order to notify Skia that we've updated handles within the current binding and invalidate any assumptions about previous modifications it has made to the context. This reverts commit d6e5946.
|
While it's similar to the font corruption, it's not quite the same issue. That corruption was happening for two reasons. First, because of how the texture indexing was being passed into the shader, sometimes it would access the wrong atlas texture. Second, when we recycled areas in the atlas sometimes we uploaded over an area that was still in use. Both of these were internal to the font system so nothing external was stomping textures. |
|
Replaced by #25349 |
…lutter#25292) This will cause the previous texture binding used by Skia to be incorrectly unbound, such that should they need to re-used an existing binding, we'll end up rendering blank instead. Instead we should be using context->flushAndSubmit(); context->resetContext(kAll_GrBackendState); in `EmbedderExternalTextureGL::ResolveTexture` in order to notify Skia that we've updated handles within the current binding and invalidate any assumptions about previous modifications it has made to the context. This reverts commit d6e5946.
3 participants
Fixes flutter/flutter#78648