Plotter: Scope GL Program to lifespan of scenegraph node
ClosedPublic

Authored by davidedmundson on Apr 18 2019, 10:17 AM.

Details

Summary

Currently the QOpenGLProgram was static. This works when you only have
one OpenGL context that is never invalidated.

Instead we should have a new program created for each context. There is
no benefit of being static when we can use the cached shader loading.

As we need a program per context, we would need to handle windowChanged
and sceneGraphInvalidated manually. Instead we can scope the program to
the QSGNode which will be deleted and recreated on the render thread
automatically by the scene graph backend.

We can also drop ManagedTextureNode and use
QSGSimpleTextureNode::setOwnsTexture which does the same thing.

BUG: 403453

Test Plan

Created a CPU load viewer on my panel
Dragged it to my desktop
Previously that didn't render anything, now it works

I am quite confident it will fix the crashes that we see on window moves and handling sceneGraphInvalidated

Diff Detail

Repository
R296 KDeclarative
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 18 2019, 10:17 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidedmundson requested review of this revision.Apr 18 2019, 10:17 AM
davidedmundson edited the summary of this revision. (Show Details)Apr 18 2019, 10:18 AM
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: dfaure.
broulik accepted this revision.Apr 18 2019, 3:50 PM
broulik added a subscriber: broulik.
broulik added inline comments.
src/qmlcontrols/kquickcontrolsaddons/plotter.cpp
267 ↗(On Diff #56505)

Coding style

This revision is now accepted and ready to land.Apr 18 2019, 3:50 PM

Thanks, this fixes the plasma crash on resume.

[KMail's webengine view still displays garbage on resume, but that's less critical, and unrelated to this patch]

This revision was automatically updated to reflect the committed changes.

Thanks, this fixes the plasma crash on resume.

Thanks for the fast reports / testing.

[KMail's webengine view still displays garbage on resume, but that's less critical, and unrelated to this patch]

Is it worse than before any of the QSurfaceFormat::Reset flag stuff?

No, it's pretty much the same. The garbage looks a bit different, but switching to another email and back fixes it, so no big deal. I thought this was what this whole effort was about, though :-)