Summary: | [WebGL] Limit maximum texture sizes on older Intel hardware | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||
Component: | WebGL | Assignee: | Dean Jackson <dino> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, kondapallykalyan, noam, roger_fong | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dean Jackson
2014-04-18 17:05:25 PDT
Created attachment 229686 [details]
Patch
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? (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. Committed r167520: <http://trac.webkit.org/changeset/167520> 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). s/canted/changed/ |