Skip to content

Only call eglMakeCurrent if anything has changed in CurrentContextGuard::drop#360

Open
mrobinson wants to merge 1 commit into
servo:mainfrom
mrobinson:lightweight-context-guard
Open

Only call eglMakeCurrent if anything has changed in CurrentContextGuard::drop#360
mrobinson wants to merge 1 commit into
servo:mainfrom
mrobinson:lightweight-context-guard

Conversation

@mrobinson
Copy link
Copy Markdown
Member

Instead of unconditionally calling eglMakeCurrent when restoring the
context in CurrentContextGuard::drop only do this if anything has
changed. This guards against drivers that do too much when calling
eglMakeCurrent.

Comment thread src/platform/generic/egl/context.rs Outdated

// Only call eglMakeCurrent again if anything has changed.
if egl.GetCurrentDisplay() == self.egl_display
&& egl.GetCurrentSurface(egl::DRAW as EGLint) != self.old_egl_draw_surface
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using != for two if these checks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I accidentally upload an old version of this change. This should be addressed now.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jun 3, 2026

What is the effect of these changes?

…Guard::drop`

Instead of unconditionally calling `eglMakeCurrent` when restoring the
context in `CurrentContextGuard::drop` only do this if anything has
changed. This guards against drivers that do too much when calling
`eglMakeCurrent`.

Signed-off-by: Martin Robinson <mrobinson@abandonedwig.info>
@mrobinson mrobinson force-pushed the lightweight-context-guard branch from 24c6928 to 3057231 Compare June 3, 2026 17:55
@mrobinson
Copy link
Copy Markdown
Member Author

What is the effect of these changes?

In the end I think this depends on what the driver does when you call eglMakeCurrent with a set of arguments that represents the current state. I created this change as part of tracking down spurious calls to eglMakeCurrent while fixing servo/servo#30827. In the end this change didn't fix the issue, but it seems that we should avoid calling eglMakeCurrent again if we don't actually need to, as we are then leaving it up to the driver to be well-behaved.

@jdm jdm enabled auto-merge June 3, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants