[Webkit-unassigned] [Bug 155536] Wrong use of EGL_DEPTH_SIZE

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=155536

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #8)
> Comment on attachment 292062 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=292062&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        EGL_BUFFER_SIZE retuned a valid depth, it still might not be supported by the default screen and XCreatePixmap
> 
> retuned -> returned
> 
> > Source/WebCore/ChangeLog:12
> > +        EGL config, to avoid failures when creating the pixmap. So, We can use EGL_NATIVE_VISUAL_ID instead, and
> 
> We -> we
> 
> > Source/WebCore/ChangeLog:28
> > +        the depthj to be passed to XCreatePixmap. Also use the XErrorTrapper class to ignore all BadDrawable errors
> 
> depthj -> depth
> 
> > Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:210
> > +    // Some drivers fail to create the surface producing BadDrawable X error and the default XError handler normally aborts.
> > +    // However, if the X error is ignored, eglCreatePixmapSurface() ends up returning a surface and we can continue creating
> > +    // the context. Since this is an offscreen context, it doesn't matter if the pixmap used is not valid because we never do
> > +    // swap buffers. So, we use a custom XError handler here that ignores BadDrawable errors and only warns about any other
> > +    // errors without aborting in any case.
> > +    XErrorTrapper trapper(x11Display, XErrorTrapper::Policy::Warn, { BadDrawable });
> 
> This seems like a workaround to a broken crap proprietary graphics driver.
> Are you sure we really want to upstream this workaround? IMO this should be
> carried downstream in the project for the client that made the mistake of
> using a proprietary graphics stack.

It's a workaround for a buggy driver, but also a better way to handle X errors in general. You can see other libs using X like GTK+ itself, that are full of blocks trapping X errors. I tried to be conservative here, because I don't want an error trapper like this to hide real bugs, but the plan is to use it every time we detect an X error crashing the web process for no reason and we know it's not a our fault. So, yes I want to upstream this.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161019/ed4a2f08/attachment.html>