Issue 11467008: Always use glXCreateContextAttribsARB to create GLX contexts.

Issue 11467008: Always use glXCreateContextAttribsARB to create GLX contexts. (Closed)

Created:
8 years ago by ccameron

Modified:
8 years ago

Reviewers:
Ken Russell (switch to Gerrit), piman

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Explicitly create the GLX window for onscreen surfaces. Always use glXCreateContextAttribsARB to create GLX contexts. This will allow us to explicitly destroy hibernated GLX windows, which will hopefully plug some GPU memory leaks that were causing instability. BUG=145600 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=171679

Patch Set 1 #

Patch Set 2 : Rename gl_window to glx_window #

Total comments: 2

Patch Set 3 : Incorporate review feedback #

Total comments: 2

Patch Set 4 : Incorporate review feedback" #

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+37 lines, -81 lines)			Patch
M	ui/gl/gl_context_glx.cc	View		3 chunks	+17 lines, -76 lines	0 comments	Download
M	ui/gl/gl_surface_glx.h	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
M	ui/gl/gl_surface_glx.cc	View	1 2 3	5 chunks	+19 lines, -5 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

piman

LGTM+nit. Let's land this, and see if anything breaks. https://codereview.chromium.org/11467008/diff/2001/ui/gl/gl_surface_glx.h File ui/gl/gl_surface_glx.h (right): https://codereview.chromium.org/11467008/diff/2001/ui/gl/gl_surface_glx.h#newcode79 ui/gl/gl_surface_glx.h:79: ...

8 years ago (2012-12-06 20:44:06 UTC) #2

ccameron

Thanks! https://codereview.chromium.org/11467008/diff/2001/ui/gl/gl_surface_glx.h File ui/gl/gl_surface_glx.h (right): https://codereview.chromium.org/11467008/diff/2001/ui/gl/gl_surface_glx.h#newcode79 ui/gl/gl_surface_glx.h:79: gfx::AcceleratedWidget glx_window_; On 2012/12/06 20:44:06, piman wrote: > ...

8 years ago (2012-12-06 20:55:09 UTC) #3

Ken Russell (switch to Gerrit)

LGTM with a nit https://codereview.chromium.org/11467008/diff/5002/ui/gl/gl_surface_glx.cc File ui/gl/gl_surface_glx.cc (right): https://codereview.chromium.org/11467008/diff/5002/ui/gl/gl_surface_glx.cc#newcode458 ui/gl/gl_surface_glx.cc:458: NULL); You should test the ...

8 years ago (2012-12-06 21:41:20 UTC) #4

ccameron

Thanks! https://codereview.chromium.org/11467008/diff/5002/ui/gl/gl_surface_glx.cc File ui/gl/gl_surface_glx.cc (right): https://codereview.chromium.org/11467008/diff/5002/ui/gl/gl_surface_glx.cc#newcode458 ui/gl/gl_surface_glx.cc:458: NULL); On 2012/12/06 21:41:20, kbr wrote: > You ...

8 years ago (2012-12-06 22:18:10 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ccameron@chromium.org/11467008/12001

8 years ago (2012-12-06 22:23:56 UTC) #6

Message was sent while issue was closed.

Change committed as 171679

Expand Messages | Collapse Messages