Reland "RendererContextSwitch guard flutter's gl context rework. #13812" #13946
Conversation
cyanglaz
left a comment
cyanglaz
left a comment
There was a problem hiding this comment.
All the context switches added in this PR besides the re-land.
| RasterStatus Rasterizer::RasterAndSubmitCompositorFrame( | ||
| SurfaceFrame& frame, | ||
| CompositorContext::ScopedFrame& compositor_frame, | ||
| flutter::LayerTree& layer_tree) { | ||
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return RasterStatus::kFailed; | ||
| } | ||
| RasterStatus raster_status = compositor_frame.Raster(layer_tree, false); | ||
| if (raster_status == RasterStatus::kFailed) { | ||
| return raster_status; | ||
| } | ||
| frame.Submit(); | ||
| return raster_status; | ||
| } | ||
|
|
There was a problem hiding this comment.
Added switch before Raster
shell/common/rasterizer.cc
Outdated
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return RasterStatus::kFailed; | ||
| } |
There was a problem hiding this comment.
Added context switch
shell/common/rasterizer.cc
Outdated
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Added context switch
shell/common/rasterizer.cc
Outdated
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Added context switch
shell/common/rasterizer.cc
Outdated
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Added context switch
shell/common/rasterizer.cc
Outdated
| std::unique_ptr<RendererContextSwitchManager::RendererContextSwitch> | ||
| context_switch = surface_->MakeRenderContextCurrent(); | ||
| if (!context_switch->GetSwitchResult()) { | ||
| return std::nullopt; | ||
| } |
There was a problem hiding this comment.
Added context switch
|
It is a bit hard to parse just the additions so I may have something but LGTM. Can you reword the title to just say something like "Reland "RendererContextSwitch guard flutter's gl context rework."" |
cyanglaz
changed the title
Done. |
|
@chinmaygarde I rebased and it is really to review. |
|
Closing for now. Will open later when I find a reviewer for this. |
3 participants
This PR is an exact re-land of "RendererContextSwitch guard flutter's gl context rework. #13812" + the fix for the image not showing issue which caused the revert. (flutter/flutter#45098)
Added RendererContextSwitches before some other gl operations.
Added a scenario test that makes sure the images are shown. The scenario tests are currently only running on simulator on CI. It needs to be run on an actual device to be sure the gl context is set correctly for image rendering. For now, I have manually tested it locally.
The effort of running the scenario test to device on CI is tracked here: flutter/flutter#43852
Preferably, A framework side golden tests for displaying an image should be added to catch the regression
All the changes in the PR besides "RendererContextSwitch guard flutter's gl context rework. #13812" is inline commented
IMPORTANT: We should only land this when flutter/flutter#45348 is resolved.