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

[bugzilla-daemon at webkit.org]

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

Dean Jackson <dino at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #242832|review?                     |review+
              Flags|                            |

--- Comment #35 from Dean Jackson <dino at apple.com> ---
Comment on attachment 242832
  --> https://bugs.webkit.org/attachment.cgi?id=242832
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" />

???

> 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 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.

> 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.

> 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?

> 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?

-- 
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/8e6046ef/attachment-0002.html>