Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add missing context switches in Rasterizer #26847

Merged
merged 1 commit into from
Jun 20, 2021

Conversation

knopp
Copy link
Member

@knopp knopp commented Jun 20, 2021

Fixes flutter/flutter#84930

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.
  • The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label Jun 20, 2021
@google-cla google-cla bot added the cla: yes label Jun 20, 2021
@knopp knopp changed the title Add missing context swiches in Rasterizer Add missing context switches in Rasterizer Jun 20, 2021
@knopp knopp force-pushed the rasterizer_wrong_glcontexts branch from a5754c1 to c3b2324 Compare June 20, 2021 08:53
@knopp knopp changed the title Add missing context switches in Rasterizer WIP: Add missing context switches in Rasterizer Jun 20, 2021
@knopp knopp force-pushed the rasterizer_wrong_glcontexts branch from c3b2324 to 135a30a Compare June 20, 2021 09:16
@knopp knopp changed the title WIP: Add missing context switches in Rasterizer Add missing context switches in Rasterizer Jun 20, 2021
@knopp knopp merged commit 759341e into flutter:master Jun 20, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 20, 2021
wqyfavor pushed a commit to wqyfavor/engine that referenced this pull request Jun 21, 2021
@knopp knopp deleted the rasterizer_wrong_glcontexts branch July 17, 2021 09:24
moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
Comment on lines +151 to +154
auto context_switch = surface_->MakeRenderContextCurrent();
if (!context_switch->GetResult()) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be wrong.

This means that if the OS sends a low memory warning, we will fail to clear the cache if we're unable to make the context current. This will result in the OS killing us or some other process(es), particularly on mobile/Fuchsia/CastOS.

If we're running low on memory, we should unconditionally purge the resource cache. This will not result in context corruption, though it may slow down next frames.

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed flutter/flutter#105192 to discuss

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes embedder Related to the embedder API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux: Creating new engine corrupts OpenGL context in other engines
3 participants