Bug 131867 – [WebGL] Limit maximum texture sizes on older Intel hardware

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Dean Jackson]

Chrome and Firefox limit the maximum size of textures on Intel hardware due to corruption that may reveal the contents of other windows. We should do the same.

<rdar://problem/16582628>

[Dean Jackson]

Created attachment 229686 [details]
Patch

[Geoffrey Garen]

Comment on attachment 229686 [details]
Patch

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

r=me

> Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLCommon.cpp:103
> +    // Intel HD 3000 devices have problems with
> +    // large textures. <rdar://problem/16649140>

This can be one line.

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:252
> +    case MAX_TEXTURE_SIZE:
> +        ::glGetIntegerv(MAX_TEXTURE_SIZE, value);
> +        if (getExtensions()->requiresRestrictedMaximumTextureSize())
> +            *value = std::min(4096, *value);
> +        break;
> +    case MAX_CUBE_MAP_TEXTURE_SIZE:
> +        ::glGetIntegerv(MAX_CUBE_MAP_TEXTURE_SIZE, value);
> +        if (getExtensions()->requiresRestrictedMaximumTextureSize())
> +            *value = std::min(1024, *value);
> +        break;

Should we really silently truncate instead of just failing and returning an error? What's the expected behavior here?

[Dean Jackson]

(In reply to comment #2)
> (From update of attachment 229686 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229686&action=review
> 
> r=me
> 
> > Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLCommon.cpp:103
> > +    // Intel HD 3000 devices have problems with
> > +    // large textures. <rdar://problem/16649140>
> 
> This can be one line.
> 
> > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:252
> > +    case MAX_TEXTURE_SIZE:
> > +        ::glGetIntegerv(MAX_TEXTURE_SIZE, value);
> > +        if (getExtensions()->requiresRestrictedMaximumTextureSize())
> > +            *value = std::min(4096, *value);
> > +        break;
> > +    case MAX_CUBE_MAP_TEXTURE_SIZE:
> > +        ::glGetIntegerv(MAX_CUBE_MAP_TEXTURE_SIZE, value);
> > +        if (getExtensions()->requiresRestrictedMaximumTextureSize())
> > +            *value = std::min(1024, *value);
> > +        break;
> 
> Should we really silently truncate instead of just failing and returning an error? What's the expected behavior here?

Yeah, we need to truncate. If we just fail then Intel 3000 cards can never render with textures, which breaks WebGL pretty completely.

A limit of 4096*4096 is still a huge texture, larger than most screens. It's not an overly restrictive limit.

[Dean Jackson]

Committed r167520: <http://trac.webkit.org/changeset/167520>

[Alexey Proskuryakov]

This broke two tests on bots:
fast/canvas/webgl/webgl-compressed-texture-size-limit.html
webgl/1.0.2/conformance/textures/texture-size-limit.html

Given that the tests used to pass on the hardware we have, seems like the patch may be over-zealous? Failing a conformance test doesn't seem great either (we actually fail it, it's not just canted output).

[Alexey Proskuryakov]

s/canted/changed/