Differential D26556

[scenes/opengl] Remove outdated hack to reset vertex buffers
ClosedPublic
Actions

Authored by davidedmundson on Jan 10 2020, 1:17 AM.

Details

Reviewers
zzag
Group Reviewers
KWin
Commits
R108:212d87a32ef1: [scenes/opengl] Remove outdated hack to reset vertex buffers
Summary

Scene opengl has a callback for when we have a GL error. One of the
handlers for an error calls scheduleVboReInit the history shows it was a
forerunner to the GLX_NV_robustness_video_memory_purge but resetting
only one tiny part based on debug output.

When we get here we schedule a reset of the vertex buffer, via a timer.
When the timer is caled we have no idea what GL context was last
current, if it's not the currect context then the main scene
GLVertexBuffer will be deleted but not correctly re-initialised.

We have two very common crashes with a corrupted
GLVertexBuffer::streamingBuffer() which would match up perfectly.

Given that we now have a proper mechanism to reset the entire scene, we
don't need this timer based hack and resolve that problem.

BUG: 399499
BUG: 372305

Test Plan

I could never reproduce the crash, but it's a good theory at least.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Jan 10 2020, 1:17 AM
Restricted Application added a project: KWin. · View Herald TranscriptJan 10 2020, 1:17 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Jan 10 2020, 1:17 AM
davidedmundson added a comment.Jan 10 2020, 1:22 AM

Extra clue that matches my thinking:

https://bugs.kde.org/show_bug.cgi?id=399499#c46

kwin_scene_opengl: 0x0: OpenGL debug output initialized

which shows the reporter at least one reporter is potentially in this path

davidedmundson edited the summary of this revision. (Show Details)Jan 10 2020, 1:24 AM
davidedmundson updated this revision to Diff 73180.Jan 10 2020, 10:32 AM

Add extra guard, as there is a timer there's a hypothetical (but not real) chance that scene is destroyed

davidedmundson added a comment.Jan 10 2020, 11:11 AM

As a more general comment, I think this entire debug context stuff can be deleted or at least opt-in when we want debug, rather than what seems to be a poor approach of detecting the nvidia resets.

But I want to get the simpler fix in first, maybe even into 5.12

zzag added a subscriber: zzag.Jan 10 2020, 1:41 PM
zzag added inline comments.
plugins/scenes/opengl/scene_opengl.cpp
379–383

Was there any reason not to use gl_debuggedScene directly?

davidedmundson updated this revision to Diff 73201.Jan 10 2020, 2:21 PM
davidedmundson retitled this revision from [scenes/opengl] Set current context when resetting common vertex buffer to [scenes/opengl] Remove outdated hack to reset vertex buffers.
davidedmundson edited the summary of this revision. (Show Details)
davidedmundson removed a subscriber: zzag.

Just remove the whole thing

zzag added inline comments.Jan 10 2020, 2:24 PM
plugins/scenes/opengl/scene_opengl.cpp
453–454

Please remove these two lines as well.

davidedmundson marked an inline comment as done.Jan 10 2020, 2:25 PM
zzag accepted this revision.Jan 10 2020, 2:43 PM

+1, less hacky code.

This revision is now accepted and ready to land.Jan 10 2020, 2:43 PM
Closed by commit R108:212d87a32ef1: [scenes/opengl] Remove outdated hack to reset vertex buffers (authored by davidedmundson). · Explain WhyJan 10 2020, 2:51 PM
This revision was automatically updated to reflect the committed changes.
davidedmundson marked an inline comment as done.

Revision Contents
Changeset List

Diff 73203

View Options

plugins/scenes/opengl/scene_opengl.cpp

Loading...