[Linux] Move compositor shader into its own GObject#188144
Conversation
Extracts the OpenGL shader program used by FlCompositorOpenGL into a new FlCompositorOpenGLShader GObject. This encapsulates the shader sources, program, uniform locations and vertex buffer, along with their compilation and cleanup, behind a small API.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. 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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request refactors the Linux platform's OpenGL compositor by extracting shader compilation, binding, and uniform configuration logic from FlCompositorOpenGL into a new dedicated class, FlCompositorOpenGLShader. Feedback on these changes focuses on improving robustness: checking for null pointers during dispose to prevent crashes on subsequent calls, validating constructor arguments, and ensuring the shader program compiled successfully before attempting to use it or set uniform values.
- Fix the vertical layer offset to divide by frame height instead of width in composite_layer. - Make fl_compositor_opengl_shader_dispose safe to run multiple times by guarding OpenGL cleanup on the OpenGL manager being set. - Add a g_return_val_if_fail check to fl_compositor_opengl_shader_new.
mattkae
left a comment
There was a problem hiding this comment.
Solid change! I have a suggestion, but nothing blocking
flutter/flutter@e228771...87224e0 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 5cae7f9ada62 to 3a66ea7b9aaa (1 revision) (flutter/flutter#188379) 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 1e6c246bb73a to 5cae7f9ada62 (2 revisions) (flutter/flutter#188370) 2026-06-23 engine-flutter-autoroll@skia.org Roll Skia from 766f21ae61dc to ffac3e91fbc7 (24 revisions) (flutter/flutter#188366) 2026-06-23 737941+loic-sharma@users.noreply.github.com [Windows] Add public API to post task to platform thread (flutter/flutter#187365) 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 7ab0179ce4d4 to 1e6c246bb73a (1 revision) (flutter/flutter#188354) 2026-06-23 robert.ancell@canonical.com Fix byte/character offset confusion in FlAccessibleTextField (flutter/flutter#188138) 2026-06-22 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Lm76V7lvxVA0r1De5... to RymJjIj7dd5vQ3Cnh... (flutter/flutter#188353) 2026-06-22 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#188355) 2026-06-22 154381524+flutteractionsbot@users.noreply.github.com Sync CHANGELOG.md from stable (flutter/flutter#188331) 2026-06-22 engine-flutter-autoroll@skia.org Roll Skia from 5fbb9bbd889c to 766f21ae61dc (2 revisions) (flutter/flutter#188184) 2026-06-22 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 6.0.3 to 7.0.0 in the all-github-actions group (flutter/flutter#188350) 2026-06-22 robert.ancell@canonical.com Use g_signal_connect_object in the Linux embedder (flutter/flutter#188241) 2026-06-22 robert.ancell@canonical.com Disconnect from parent window signal when view is destroyed (flutter/flutter#185521) 2026-06-22 rmacnak@google.com Remove many absolute paths from build commands. (flutter/flutter#187765) 2026-06-22 haiderqadir.hq@gmail.com Fix spelling mistake in documentation (wether → whether) (flutter/flutter#186141) 2026-06-22 engine-flutter-autoroll@skia.org Roll Dart SDK from a748c4b15399 to 7ab0179ce4d4 (2 revisions) (flutter/flutter#188332) 2026-06-22 robert.ancell@canonical.com [Linux] Move compositor shader into its own GObject (flutter/flutter#188144) 2026-06-22 bkonyi@google.com Add agent skills for orchestrating cherry-picks to stable and beta channels (flutter/flutter#187860) 2026-06-22 engine-flutter-autoroll@skia.org Roll Packages from c516c92 to cd5194a (1 revision) (flutter/flutter#188312) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Extracts the OpenGL shader program used by FlCompositorOpenGL into a new FlCompositorOpenGLShader GObject. This encapsulates the shader sources, program, uniform locations and vertex buffer, along with their compilation and cleanup, behind a small API.
Extracts the OpenGL shader program used by FlCompositorOpenGL into a new FlCompositorOpenGLShader GObject. This encapsulates the shader sources, program, uniform locations and vertex buffer, along with their compilation and cleanup, behind a small API.