[Webkit-unassigned] [Bug 109332] EXT_sRGB needs implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=109332

--- Comment #36 from Roger Fong <roger_fong at apple.com> ---
(In reply to comment #35)
> Comment on attachment 242832 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242832&action=review
> 
> > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:14951
> > -    <ClCompile Include="..\editing\win\EditorWin.cpp"/>
> > +    <ClCompile Include="..\editing\win\EditorWin.cpp" />
> 
> Oops.
> 
> > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:7263
> > +    <ClCompile Include="..\page\PageConfiguration.cpp" />
> > +    <ClCompile Include="..\loader\cache\CacheValidation.cpp" />
> > +    <ClCompile Include="..\html\HTMLWBRElement.cpp" />
> 
> ???
> 
> > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:15271
> > +    <ClInclude Include="..\loader\cache\CacheValidation.h" />
> > +    <ClInclude Include="..\html\HTMLWBRElement.h" />
> > +    <ClInclude Include="..\storage\StorageNamespaceProvider.h" />
> 
> ???

Hmm, I think visual studio is doing some funky things.
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:15799
> > +				5C4304AD191AC908000E2BC0 /* EXTShaderTextureLOD.cpp */,
> > +				5C4304AE191AC908000E2BC0 /* EXTShaderTextureLOD.h */,
> > +				5C4304AF191AC908000E2BC0 /* EXTShaderTextureLOD.idl */,
> 
> Was this intentional?
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:18734
> > +				5C4304B3191AEF46000E2BC0 /* JSEXTShaderTextureLOD.cpp */,
> > +				5C4304B4191AEF46000E2BC0 /* JSEXTShaderTextureLOD.h */,
> 

And maybe Xcode too?...I'll remove those lines and see if things still work.
> And here?
> 
> > Source/WebCore/html/canvas/EXTsRGB.cpp:4
> > +/*
> > + * Copyright (C) 2012 Google Inc. All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> 
> Did you really take this from Google's implementation? Or did you copy an
> existing file? If the latter, then the copyright should be Apple.

I just copy pasted the header from a different file. Will switch.

> 
> > Source/WebCore/html/canvas/EXTsRGB.h:2
> > + * Copyright (C) 2012 Google Inc. All rights reserved.
> 
> Same here.
> 
> > Source/WebCore/html/canvas/EXTsRGB.idl:29
> > +NoInterfaceObject,
> > +Conditional=WEBGL,
> > +GenerateIsReachable=ImplWebGLRenderingContext
> 
> We indent these lines. I don't know why.

Ok

> 
> > Source/WebCore/html/canvas/WebGLFramebuffer.cpp:436
> > +        // Attaching an SRGB_EXT format attachment to a framebuffer is invalid
> 
> Nit: missing period.
> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2631
> > +        case Extensions3D::FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING_EXT:
> > +            {
> > +                WebGLRenderbuffer* renderBuffer = reinterpret_cast<WebGLRenderbuffer*>(object);
> > +                GC3Denum renderBufferFormat = renderBuffer->getInternalFormat();
> > +                ASSERT(renderBufferFormat != Extensions3D::SRGB_EXT && renderBufferFormat != Extensions3D::SRGB_ALPHA_EXT);
> > +                if (renderBufferFormat == Extensions3D::SRGB8_ALPHA8_EXT)
> > +                    return WebGLGetInfo(Extensions3D::SRGB_EXT);
> > +                return WebGLGetInfo(GraphicsContext3D::LINEAR);
> > +            }
> 
> One level too much indentation here, and the opening { goes on the case line.
> 
> If renderBufferFormat is not allowed to be SRGB_ALPHA_EXT, why don't you
> check for that as well in WebGLFramebuffer.cpp?

I don't think that a value of SRGB_ALPHA_ExT could ever get to this code path. But I suppose it couldn't hurt to check it in multiple places.

> 
> > Source/WebCore/platform/graphics/opengl/Extensions3DOpenGL.cpp:174
> > +    if (name == "GL_EXT_sRGB")
> > +        return m_availableExtensions.contains("GL_EXT_texture_sRGB") && (m_availableExtensions.contains("GL_EXT_framebuffer_sRGB") || m_availableExtensions.contains("GL_ARB_framebuffer_sRGB"));
> > +
> 
> Does this work on iOS?

I've completely neglected my iOS testing. Will do that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141208/b335431f/attachment-0002.html>