Skip to content

Conversation

@guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Nov 7, 2025

The Frame Buffer Object (FBO) feature is part of OpenGL since version 3.0 from 2008. Nowadays, we can therefore assume that this feature is always available. That means we don't have to use GLEW to check whether this feature is present.

Like that, we can avoid using GLEW entirely, which is an implementation detail of OpenGL loading in ROOT that will soon go away.

To make sure GLEW is not included anymore, this commit replaces includes of TGLIncludes.h with the OpenGL headers only, because TGLIncludes.h was including OpenGL plus GLEW.

See also:

FYI @smuzaffar @bellenot @osschar

The Frame Buffer Object (FBO) feature is part of OpenGL since version
3.0 from 2008. Nowadays, we can therefore assume that this feature
is always available. That means we don't have to use GLEW to check
whether this feature is present.

Like that, we can avoid using GLEW entirely, which is an implementation
detail of OpenGL loading in ROOT that will soon go away.

To make sure GLEW is not included anymore, this commit replaces includes
of `TGLIncludes.h` with the OpenGL headers only, because `TGLIncludes.h`
was including OpenGL plus GLEW.

See also:

  * root-project/root#18471
  * root-project/root#20294 (comment)
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2025

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49341/46717

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2025

A new Pull Request was created by @guitargeek for master.

It involves the following packages:

  • Fireworks/Core (visualization)
  • Fireworks/Vertices (visualization)

@Dr15Jones, @alja, @cmsbuild, @makortel can you please review it and eventually sign? Thanks.
@alja this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2025

+1

Size: This PR adds an extra 48KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-743022/49337/summary.html
COMMIT: 8305656
CMSSW: CMSSW_16_0_X_2025-11-06-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49341/49337/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • Reco comparison had 2 failed jobs
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3939953
  • DQMHistoTests: Total failures: 53
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3939880
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 218 log files, 188 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@ajaychandrak2002-bit
Copy link

if (GLEW_EXT_framebuffer_object) {
// Multi-threaded save
futures.push_back((*j)->CaptureAndSaveImage(file, height));
} else {
// Single-threaded save
if (height == -1)
(*j)->GetGLViewer()->SavePicture(file);
else
(*j)->GetGLViewer()->SavePictureHeight(file, height);

there are some bugs in this programs and please find and fix it properly. there are no extint

@osschar
Copy link
Contributor

osschar commented Nov 7, 2025

Here the TGLIncludes also shielded us for OSX ... Fireworks/ packages are part of FWLite and we used to build binary releases of the event-display for mac and linux.

I assume we will not attempt any new mac builds ... so this is all fine :)

@guitargeek
Copy link
Contributor Author

Thanks for the comment 👍 I was suspecting that macOS was the original reason

@osschar
Copy link
Contributor

osschar commented Nov 8, 2025

Well, the original reason was it took forever for lxplus (and other HEP community resources) to upgrade to slc-6 (then in-house rhel-6 derivative, which shipped with mesa-gl that provided FBOs etc) and we needed things to work there as well as work really well on machines that already had proper GL support, in this case in particular, in the CMS control room. But I'm giving you a history lesson here 🙂

@smuzaffar
Copy link
Contributor

@alja , you need to use +1 comment to approve the change. cms-bot does not understand the review approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants