Bug 202159 - Add ANGLE backend for iOS device
Summary: Add ANGLE backend for iOS device
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks: 198948
  Show dependency treegraph
 
Reported: 2019-09-24 12:45 PDT by Dean Jackson
Modified: 2019-11-09 11:59 PST (History)
19 users (show)

See Also:


Attachments
WIP (24.99 KB, patch)
2019-09-24 12:46 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (325.50 KB, patch)
2019-10-31 15:52 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (171.86 KB, patch)
2019-11-07 15:05 PST, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (164.71 KB, patch)
2019-11-07 15:17 PST, Dean Jackson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2019-09-24 12:45:50 PDT
Compile ANGLE backend for iOS device
Comment 1 Dean Jackson 2019-09-24 12:46:52 PDT
Created attachment 379471 [details]
WIP
Comment 2 Dean Jackson 2019-10-31 15:52:43 PDT
Created attachment 382513 [details]
Patch
Comment 3 Dean Jackson 2019-11-07 15:05:51 PST
Created attachment 383077 [details]
Patch
Comment 4 Radar WebKit Bug Importer 2019-11-07 15:06:24 PST
<rdar://problem/57000166>
Comment 5 Dean Jackson 2019-11-07 15:17:02 PST
Created attachment 383082 [details]
Patch
Comment 6 Build Bot 2019-11-07 15:18:15 PST
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 7 Myles C. Maxfield 2019-11-07 15:23:21 PST
Comment on attachment 383077 [details]
Patch

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

Cursory look seems okay. This is the beginning of a big project so it's difficult to be thorough...

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/ContextEAGL.h:44
> +class ContextEAGL : public ContextGL
> +{
> +  public:
> +    ContextEAGL(const gl::State &state,
> +               gl::ErrorSet *errorSet,
> +               const std::shared_ptr<RendererGL> &renderer);
> +};

Is this a stub for something that will come later? There's nothing here in this class.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/DisplayEAGL.mm:166
> +    return new IOSurfaceSurfaceEAGL(state, mContext, clientBuffer, attribs);

This seems so leaky :( :( :(

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/DisplayEAGL.mm:205
> +    config.nativeVisualID   = 0;
> +    config.nativeVisualType = 0;
> +    config.nativeRenderable = EGL_TRUE;

I'd appreciate a link for where these numbers came from

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/IOSurfaceSurfaceEAGL.mm:309
> +#pragma clang diagnostic push
> +#pragma clang diagnostic ignored "-Wunused-variable"

This file is iOS-specific. Can't we just fix the problem instead of ignoring warnings?
Comment 8 Dean Jackson 2019-11-07 15:50:09 PST
Comment on attachment 383077 [details]
Patch

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

>> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/ContextEAGL.h:44
>> +};
> 
> Is this a stub for something that will come later? There's nothing here in this class.

Yeah. I once had a FIXME in here saying that ContextEAGL and ContextCGL could go away.

>> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/DisplayEAGL.mm:166
>> +    return new IOSurfaceSurfaceEAGL(state, mContext, clientBuffer, attribs);
> 
> This seems so leaky :( :( :(

It does!

We release it from the calling side though.

>> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/DisplayEAGL.mm:205
>> +    config.nativeRenderable = EGL_TRUE;
> 
> I'd appreciate a link for where these numbers came from

I don't think these matter for this implementation. If they are the default values, I should just remove these lines.

The values below are pretty obvious, I hope.

>> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/IOSurfaceSurfaceEAGL.mm:309
>> +#pragma clang diagnostic ignored "-Wunused-variable"
> 
> This file is iOS-specific. Can't we just fix the problem instead of ignoring warnings?

The problem is the iOS Simulator doesn't have texImageIOSurface, and ANGLE doesn't have something like the UNUSED_PARAM macro (as far as I can tell).
Comment 9 James Darpinian 2019-11-07 16:30:26 PST
Comment on attachment 383077 [details]
Patch

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

>>> Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/eagl/IOSurfaceSurfaceEAGL.mm:309
>>> +#pragma clang diagnostic ignored "-Wunused-variable"
>> 
>> This file is iOS-specific. Can't we just fix the problem instead of ignoring warnings?
> 
> The problem is the iOS Simulator doesn't have texImageIOSurface, and ANGLE doesn't have something like the UNUSED_PARAM macro (as far as I can tell).

There's a macro ANGLE_UNUSED_VARIABLE in angle/src/common/debug.h
Comment 10 Dean Jackson 2019-11-08 11:01:33 PST
Committed r252245: <https://trac.webkit.org/changeset/252245>
Comment 11 Dean Jackson 2019-11-09 11:59:29 PST
Committed r252250: <https://trac.webkit.org/changeset/252250>