Bug 131867 - [WebGL] Limit maximum texture sizes on older Intel hardware
Summary: [WebGL] Limit maximum texture sizes on older Intel hardware
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-18 17:05 PDT by Dean Jackson
Modified: 2014-04-18 22:32 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.62 KB, patch)
2014-04-18 17:09 PDT, Dean Jackson
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2014-04-18 17:05:25 PDT
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>
Comment 1 Dean Jackson 2014-04-18 17:09:37 PDT
Created attachment 229686 [details]
Patch
Comment 2 Geoffrey Garen 2014-04-18 17:17:31 PDT
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?
Comment 3 Dean Jackson 2014-04-18 17:20:15 PDT
(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.
Comment 4 Dean Jackson 2014-04-18 17:30:40 PDT
Committed r167520: <http://trac.webkit.org/changeset/167520>
Comment 5 Alexey Proskuryakov 2014-04-18 22:23:35 PDT
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).
Comment 6 Alexey Proskuryakov 2014-04-18 22:32:36 PDT
s/canted/changed/