gl: make shared OpenGL infrastructure profile-safe - #16
Open
tritao wants to merge 4 commits into
Open
Conversation
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 7, 2026 23:57
0f14f15 to
f09c6b7
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 00:17
f09c6b7 to
438c680
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 00:42
438c680 to
9d5aa7b
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 01:07
9d5aa7b to
17f663a
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 01:20
17f663a to
5534301
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 01:22
5534301 to
1898f72
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 01:24
1898f72 to
00e03cf
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 01:30
00e03cf to
32b1cf7
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
2 times, most recently
from
August 8, 2026 01:57
e850c17 to
0503981
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 02:04
0503981 to
4b9f662
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 02:32
4b9f662 to
9e20664
Compare
tritao
marked this pull request as ready for review
August 8, 2026 02:36
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 02:54
9e20664 to
654a702
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 8, 2026 10:27
0af09b0 to
96e1679
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 9, 2026 02:03
eb617ed to
7a2272e
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 9, 2026 02:35
7a2272e to
4f38785
Compare
This was referenced Aug 9, 2026
tritao
force-pushed
the
stack/compatibility-boundary
branch
3 times, most recently
from
August 9, 2026 11:37
777ae7b to
293984a
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
2 times, most recently
from
August 9, 2026 12:02
b1748c3 to
0f0c8b0
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
2 times, most recently
from
August 9, 2026 14:55
79e6932 to
626fb23
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
from
August 9, 2026 15:10
626fb23 to
8083b63
Compare
tritao
force-pushed
the
stack/compatibility-boundary
branch
2 times, most recently
from
August 9, 2026 21:01
a8f3ebf to
bc0c4c3
Compare
Lgt2x
reviewed
Aug 19, 2026
Comment on lines
82
to
+85
| - name: Build EGL binding test | ||
| run: cmake --build cmake_build_dir --target EGLBindingTest GLSLRuntimeTest --config Release -- -j4 | ||
| run: cmake --build cmake_build_dir --target EGLBindingTest GLSLRuntimeTest OffscreenReadbackTest --config Release -- -j4 | ||
| - name: Run core-profile OpenGL tests | ||
| run: ctest -C Release -R '^(EGLBindingTest|GLSLRuntimeTest)$' --output-on-failure | ||
| run: ctest -C Release -R '^(EGLBindingTest|GLSLRuntimeTest|OffscreenReadbackTest)$' --output-on-failure |
Member
There was a problem hiding this comment.
I'm still unsure why we separate test runs in multiple passes and jobs instead of running all of them at once
Comment on lines
+172
to
+183
| add_executable(OffscreenReadbackTest OffscreenReadbackTest.cpp) | ||
| target_link_libraries(OffscreenReadbackTest PRIVATE | ||
| CoinGLTestSupport CoinGLReadbackTestSupport CoinTestInternals Coin | ||
| ${COIN_TARGET_LINK_LIBRARIES}) | ||
| target_include_directories(OffscreenReadbackTest PRIVATE | ||
| ${PROJECT_SOURCE_DIR}/src | ||
| ${PROJECT_SOURCE_DIR}/include | ||
| ${PROJECT_BINARY_DIR}/include | ||
| ${COIN_TARGET_INCLUDE_DIRECTORIES}) | ||
| add_test(NAME OffscreenReadbackTest COMMAND OffscreenReadbackTest) | ||
| set_tests_properties(OffscreenReadbackTest PROPERTIES | ||
| SKIP_RETURN_CODE 77 LABELS "gl-core") |
Member
There was a problem hiding this comment.
we're starting to have a lot of redundancy here, you can make proper function to add gl tests 2 PRs up the stack
| run: | | ||
| sudo apt-get -y update | ||
| sudo apt-get -y install freeglut3-dev | ||
| sudo apt-get -y install freeglut3-dev libglfw3-dev |
Member
There was a problem hiding this comment.
technically it's installed automatically by the testsuite, but I prefer to install it on the system like this
Comment on lines
+78
to
+82
| #else | ||
| (void)index; | ||
| (void)point; | ||
| (void)normal; | ||
| #endif |
Member
There was a problem hiding this comment.
So if Coin is not build with legacy gl here, it's a noop? Is the class in a broken state temporarily with this PR?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make Coin's shared OpenGL infrastructure safe to execute on both compatibility and core-profile contexts.
This layer does not remove LegacyGL scene traversal or exclude LegacyGL implementation units; those boundaries are introduced next.
Stack
Layer 5/18, based on PR #22.