Bug 134854

Summary: Safari 8 on OSX 10.10 does not run WebGL in Retina HiDPI mode.
Product: WebKit Reporter: rik
Component: WebGLAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, dino, eric.carlson, esprehn+autocc, evan.exe, gyuyoung.kim, kondapallykalyan, roger_fong, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Other   
URL: http://threejs.org/examples/#webgl_buffergeometry_uint
Attachments:
Description Flags
Patch (without testcase)
none
Patch thorton: review+

Description rik 2014-07-12 06:10:04 PDT
Tested on Retina macbook pro mid 2012, nvidia GPU.

HighDPI backbuffer resolution (set to 2x the css size) does not result in a hidpi blit to the front. Changing the backbuffer resolution does change the aliasing in the picture, so that appears to do something, but it seems to be just the compositor that somehow lowers the resolution again of the final output. Antialiasing is off, so i should be able to see hard jagged poly edges at highdp, but everything is as blurry as if the output is downscaled to lowdpi and then bilinear scaled back up to the final output.
Works fine on Safari on iOS 8 on retina ipad mini and works on Chrome on OSX 10.10 on the same machine.
Canvas 2D context does seem to blit correctly with the HiDPI backbuffer res.

I just applied the latest update of 10.10, version of safari is: 8.0 (10538.39.41), bug is still present.
I cant run nightly to try there since it says its not compatible with mavericks

As a test run the threejs example in chrome and in safari and the difference is immediately visible.
http://threejs.org/examples/#webgl_buffergeometry_uint

Thanks!
Comment 1 evan.exe 2014-09-25 20:36:59 PDT
I just discovered this. This makes our app look awful, and is still present in WebKit Nightly.

One workaround is to use a CSS scale transform to do the downscaling instead of normal CSS positioning, although this goes against the recommendation from https://www.khronos.org/webgl/wiki/HandlingHighDPI.
Comment 2 Radar WebKit Bug Importer 2014-09-25 22:40:26 PDT
<rdar://problem/18465263>
Comment 3 Dean Jackson 2014-09-26 14:01:08 PDT
Yikes. This is pretty bad.
Comment 4 Dean Jackson 2014-10-03 17:52:39 PDT
I have a fix. Just need to write a test case.
Comment 5 Dean Jackson 2014-10-03 18:01:57 PDT
Created attachment 239257 [details]
Patch (without testcase)

I'll write a testcase when I think of how best to do it :)

And then submit a patch for review.
Comment 6 evan.exe 2014-10-04 07:54:58 PDT
Thanks for working on this bug! Keep in mind that the devicePixelRatio may change during the lifetime of the page. For example, the browser window may load the page on a non-retina monitor and then be dragged over to a retina monitor. It looks like your patch only reads the devicePixelRatio once on WebGL context creation time, so my guess is that it won't work correctly when the browser window is dragged between displays of different densities.
Comment 7 Dean Jackson 2014-10-06 16:48:28 PDT
(In reply to comment #6)
> Thanks for working on this bug! Keep in mind that the devicePixelRatio may change during the lifetime of the page. For example, the browser window may load the page on a non-retina monitor and then be dragged over to a retina monitor. It looks like your patch only reads the devicePixelRatio once on WebGL context creation time, so my guess is that it won't work correctly when the browser window is dragged between displays of different densities.

Nope - we don't yet do that in WebKit/Safari. We have some open bugs on it.

Also, it's not really clear if a canvas should do that when the density changes. The issue here is that the author decided they wanted a 100x100 canvas because they are on a 2x display and the canvas is 50x50 CSS px. If you want the backing store of the canvas to change then you need to adjust its width and height (effectively resetting the canvas). Images don't do this by default either. Only srcset and <picture> would handle it, because they are explicit about what content should be used at what density.

The bug was that even when the author was taking 2x into consideration, the compositor wasn't - and was still allocating a 1x rendering target.

It's an interesting question. Nothing in the canvas or WebGL spec says that anything should happen when the display density changes. That's why, for now, it should be a context creation time thing.
Comment 8 Dean Jackson 2014-10-07 17:07:07 PDT
Turns out this is untestable! (at the moment)

A script test won't work because it would read the pixels from the buffer, and the buffer is always correct. It's the compositing that is wrong.

A ref test should work, but WebGL hangs in reftests. I'll have to work out why.

Printing the layer tree also won't work, since the change is in the CALayer subclass.
Comment 9 Dean Jackson 2014-10-07 18:29:19 PDT
Created attachment 239445 [details]
Patch
Comment 10 Tim Horton 2014-10-07 18:38:20 PDT
Comment on attachment 239445 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239445&action=review

> Source/WebCore/ChangeLog:20
> +

- we don't believe in pixel tests?

> Source/WebCore/platform/graphics/GraphicsContext3D.h:445
> +            , devicePixelRatio(1.0)

just 1, I think?

> Source/WebCore/platform/graphics/mac/WebGLLayer.mm:53
> +    self.contentsScale = _devicePixelRatio;

why is iOS special here?
Comment 11 Dean Jackson 2014-10-07 18:47:01 PDT
(In reply to comment #10)
> (From update of attachment 239445 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239445&action=review

> > Source/WebCore/platform/graphics/mac/WebGLLayer.mm:53
> > +    self.contentsScale = _devicePixelRatio;
> 
> why is iOS special here?

It's not, but OS X is the only place where we render from the WebGL FBO into the CALayer backing store.

I need to investigate if we're doing this unnecessarily on OS X.
Comment 12 Dean Jackson 2014-10-07 19:23:39 PDT
Committed r174414: <http://trac.webkit.org/changeset/174414>