Bug 205520 - ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic
Summary: ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Darpinian
URL:
Keywords: InRadar
Depends on:
Blocks: webglangle
  Show dependency treegraph
 
Reported: 2019-12-20 14:28 PST by James Darpinian
Modified: 2020-03-11 17:12 PDT (History)
11 users (show)

See Also:


Attachments
ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic (6.20 KB, patch)
2019-12-20 14:30 PST, James Darpinian
no flags Details | Formatted Diff | Diff
Rebaseline layout tests (25.69 KB, patch)
2019-12-20 17:39 PST, James Darpinian
no flags Details | Formatted Diff | Diff
Remove obsolete expectations files (61.81 KB, patch)
2019-12-26 14:44 PST, James Darpinian
no flags Details | Formatted Diff | Diff
Fix one more expectations file (75.78 KB, patch)
2019-12-26 16:01 PST, James Darpinian
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Darpinian 2019-12-20 14:28:21 PST
ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic
Comment 1 James Darpinian 2019-12-20 14:30:25 PST
Created attachment 386256 [details]
ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic
Comment 2 James Darpinian 2019-12-20 17:39:05 PST
Created attachment 386283 [details]
Rebaseline layout tests
Comment 3 James Darpinian 2019-12-26 14:44:35 PST
Created attachment 386423 [details]
Remove obsolete expectations files
Comment 4 James Darpinian 2019-12-26 16:01:39 PST
Created attachment 386424 [details]
Fix one more expectations file
Comment 5 James Darpinian 2019-12-26 17:26:04 PST
Layout tests are fixed now, so this should be good to go.
Comment 6 Darin Adler 2019-12-26 19:28:04 PST
Comment on attachment 386424 [details]
Fix one more expectations file

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

I’d like to say review+ -- the part of this that corrects types from integer to float seems obviously correct -- but I don’t know enough to be sure this is exactly OK as is.

> Source/WebCore/ChangeLog:3
> +        ANGLE: Fix WebGL conformance tests for EXT_texture_filter_anisotropic

It’s a bit ambiguous or imprecise to title a patch that fixes behavior of the web engine "fix conformance tests". It’s not the tests we are fixing!

> Source/WebCore/ChangeLog:17
> +        * html/canvas/WebGL2RenderingContext.cpp:
> +        (WebCore::WebGL2RenderingContext::getExtension):
> +        (WebCore::WebGL2RenderingContext::getSupportedExtensions):
> +        (WebCore::WebGL2RenderingContext::getParameter):
> +        * html/canvas/WebGLRenderingContext.cpp:
> +        (WebCore::WebGLRenderingContext::getExtension):
> +        * html/canvas/WebGLRenderingContextBase.cpp:
> +        (WebCore::WebGLRenderingContextBase::getTexParameter):

It would be nice to have comments that explain what the changes are.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:-1372
> -    ENABLE_IF_REQUESTED(EXTTextureFilterAnisotropic, m_extTextureFilterAnisotropic, "WEBKIT_EXT_texture_filter_anisotropic", enableSupportedExtension("GL_OES_texture_float"_s));

What mitigates the risk of removing this? Is this possibly incompatible with WebKit-specific content, such as content embedded in iOS apps, that does not work with other web engines?

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:-144
> -    ENABLE_IF_REQUESTED(EXTTextureFilterAnisotropic, m_extTextureFilterAnisotropic, "WEBKIT_EXT_texture_filter_anisotropic", enableSupportedExtension("GL_EXT_texture_filter_anisotropic"_s));

What mitigates the risk of removing this? Is this possibly incompatible with WebKit-specific content, such as content embedded in iOS apps, that does not work with other web engines?
Comment 7 Dean Jackson 2019-12-27 02:04:01 PST
Comment on attachment 386424 [details]
Fix one more expectations file

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

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:-1372
>> -    ENABLE_IF_REQUESTED(EXTTextureFilterAnisotropic, m_extTextureFilterAnisotropic, "WEBKIT_EXT_texture_filter_anisotropic", enableSupportedExtension("GL_OES_texture_float"_s));
> 
> What mitigates the risk of removing this? Is this possibly incompatible with WebKit-specific content, such as content embedded in iOS apps, that does not work with other web engines?

I think it is safe to remove this. Mozilla hasn't supported their equivalent for a long time (I tried searching in their commits but failed, although I see a warning of imminent removal in 2013). Blink removed this about the same time (again, searching through their history is difficult because of file renames after the fork). A quick search on github shows three.js uses it, but only as a fallback from EXT_texture_filter_anisotropic. The WebGL conformance suite uses it in the same manner (we should remove that too).

This was one of the first ratified extensions, so it would have to be extremely old content, written when Chrome supported this.
Comment 8 WebKit Commit Bot 2019-12-27 02:50:55 PST
Comment on attachment 386424 [details]
Fix one more expectations file

Clearing flags on attachment: 386424

Committed r253919: <https://trac.webkit.org/changeset/253919>
Comment 9 WebKit Commit Bot 2019-12-27 02:50:57 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-12-27 02:51:19 PST
<rdar://problem/58215866>