Audit command buffer's context state reset code |
|||
Issue descriptionNow we have skia calling GL directly, so some previously valid assumptions are no longer valid. For example, looking at ContextState::PushTextureUnpackState(), we assumed UNPACK_SKIP_* params are always zero on the service side because we unwind them from the client side. However, now skia can just set them to none zero directly to GL, and since we don't reset them at context switch, tex upload will be wrong. I think today we are probably lucky that skia doesn't set these params. Regardless, we could either let skia have its own real context, or we need to reset all states at context switching, getting rid of whatever optimizations we may have today.
,
Nov 16
,
Nov 16
Note that these optimizations can stay when we're switching from a non-null "prev_state" ContextState to another one, but we would need to reset all the state when the "prev_state" ContextState is null (when we switch away from Skia).
,
Nov 21
,
Dec 3
FYI, note that there are additional flags: GL_UNPACK_SWAP_BYTES and GL_UNPACK_LSB_FIRST which are part of desktop GL, but not GLES. Skia doesn't currently use them (but could), worth adding these in if we try to harden context state reset code.
,
Dec 3
Since command buffer is GLES, so if we want to add something only available on desktop, we will need to define and expose an extension for that.
,
Dec 3
It's not so much to expose it to the client, but to make sure the state is reset when switching from a null ContextState (full reset).
,
Dec 3
In this case, then this audit needs to cover various desktop GL versions, various ES versions, and their exposed GL states?
,
Dec 4
Yes, or at the very least the states touched by Skia (modulo extensions that we blacklist and GL versions that we limit to). See https://cs.chromium.org/chromium/src/ui/gl/init/create_gr_gl_interface.cc?type=cs&q=ui/gl/init&sq=package:chromium&g=0&l=94 |
|||
►
Sign in to add a comment |
|||
Comment 1 by piman@chromium.org
, Nov 16