Bug 216949 – Support EXT_float_blend

• Component: WebGL
• Status: RESOLVED FIXED

[James Darpinian]

Support EXT_float_blend

[James Darpinian]

Created attachment 409636 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[James Darpinian]

Created attachment 409637 [details]
Patch

[Kenneth Russell]

Comment on attachment 409637 [details]
Patch

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

Looks good overall! The bulk of this patch is new conformance suite files which it looks like were automatically and maybe accidentally imported; please remove those so that the patch can stay better focused. Thanks.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/renderergl_utils.cpp:1485
> +                             functions->isAtLeastGLES(gl::Version(3, 2));

Double-checked the ES 3.2 spec, and the reason this is implicitly supported is that floating-point framebuffers were subsumed into the core ES 3.2 spec without the restrictions listed in EXT_color_buffer_float. Not sure whether that's worth a comment.

> Source/WebCore/html/canvas/EXTFloatBlend.cpp:2
> + * Copyright (C) 2014 Apple Inc. All rights reserved.

2020? Here and below.

> LayoutTests/webgl/resources/webgl_test_files/js/desktop-gl-constants.js:1
> +/*

Could you remove the addition of this file? It looks like it's unreferenced.

Actually it looks like many new files were accidentally imported from the top-of-tree conformance suite. Please remove them to keep this patch better focused.

[James Darpinian]

Created attachment 409741 [details]
Patch

[James Darpinian]

Comment on attachment 409637 [details]
Patch

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

>> Source/WebCore/html/canvas/EXTFloatBlend.cpp:2
>> + * Copyright (C) 2014 Apple Inc. All rights reserved.
> 
> 2020? Here and below.

Done.

>> LayoutTests/webgl/resources/webgl_test_files/js/desktop-gl-constants.js:1
>> +/*
> 
> Could you remove the addition of this file? It looks like it's unreferenced.
> 
> Actually it looks like many new files were accidentally imported from the top-of-tree conformance suite. Please remove them to keep this patch better focused.

You're right, this particular file is unnecessary. But I did intentionally import some files from top-of-tree conformance. ext-float-blend.html is not present in the 2.0.0 or 1.0.3 snapshots, so it is not automatically imported. There were already some manually imported tests, so I manually imported ext-float-blend.html alongside them, with some of the supporting files it needs.

I went through and deleted a couple of files that were unused. The rest are used by ext-float-blend.html. Hopefully in the future WebKit will import the 2.0.1 suite and then ext-float-blend.html will be automatically imported.

[EWS]

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

[James Darpinian]

Created attachment 409744 [details]
Patch

[Kenneth Russell]

Comment on attachment 409637 [details]
Patch

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

>>> LayoutTests/webgl/resources/webgl_test_files/js/desktop-gl-constants.js:1
>>> +/*
>> 
>> Could you remove the addition of this file? It looks like it's unreferenced.
>> 
>> Actually it looks like many new files were accidentally imported from the top-of-tree conformance suite. Please remove them to keep this patch better focused.
> 
> You're right, this particular file is unnecessary. But I did intentionally import some files from top-of-tree conformance. ext-float-blend.html is not present in the 2.0.0 or 1.0.3 snapshots, so it is not automatically imported. There were already some manually imported tests, so I manually imported ext-float-blend.html alongside them, with some of the supporting files it needs.
> 
> I went through and deleted a couple of files that were unused. The rest are used by ext-float-blend.html. Hopefully in the future WebKit will import the 2.0.1 suite and then ext-float-blend.html will be automatically imported.

Thanks, I understand now. Sounds fine.

[EWS]

Committed r267601: <https://trac.webkit.org/changeset/267601>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409744 [details].

[Radar WebKit Bug Importer]

<rdar://problem/69598086>

[Kenneth Russell]

*** Bug 219851 has been marked as a duplicate of this bug. ***