Bug 123915

Summary: [WebGL] We should not allow generateMipMap on compressed textures
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebGLAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, esprehn+autocc, gyuyoung.kim, kondapallykalyan, noam, roger_fong
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Bug Depends on:    
Bug Blocks: 123916    
Attachments:
Description Flags
Patch dino: review+

Description Brent Fulgham 2013-11-06 12:42:37 PST 
The WebGL conformance test suite generates an error for the Mac WebGL implementation because our implementation of glGenerateMipmaps accepts compressed textures as inputs. The specification calls for this to generate a GL INVALID_OPERATION error with the message ""trying to generate mipmaps from compressed texture".

This bug enforces the WebGL 1.0 constraint on our software.
Comment 1 Brent Fulgham 2013-11-06 13:11:41 PST 
<rdar://problem/15201274>
Comment 2 Brent Fulgham 2013-11-06 13:17:41 PST 
Created attachment 216212 [details]
Patch
Comment 3 Dean Jackson 2013-11-06 13:25:50 PST 
Comment on attachment 216212 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2227
> +#if OS(DARWIN)

Considering this is WebGL conformance, do you think this should be on all platforms?
Comment 4 Brent Fulgham 2013-11-06 13:28:22 PST 
(In reply to comment #3)
> (From update of attachment 216212 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=216212&action=review
> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2227
> > +#if OS(DARWIN)
> 
> Considering this is WebGL conformance, do you think this should be on all platforms?

Good question!  I changed from "glGenerateMipmapEXT" to "glGenerateMipmap", which might be all that is necessary on most platforms.  In those cases, GL itself should generate the error.

However, our own Apple implementation sneakily maps glGenerateMipmap -> glGenerateMipMapEXT so we can never get the desired error behavior.

It probably wouldn't hurt to remove the #if/def, since it just causes WebGL to generate the error, rather than the underlying GL implementation.

I'll go ahead and remove it while landing.
Comment 5 Brent Fulgham 2013-11-06 15:07:10 PST 
Committed r158798: <http://trac.webkit.org/changeset/158798>