Bug 227448 – [Model] [iOS] Add support for manipulating <model> inline

• Component: New Bugs
• Status: RESOLVED FIXED

[Antoine Quint]

[Model] [iOS] Add support for manipulating <model> inline

[Radar WebKit Bug Importer]

<rdar://problem/79863579>

[Antoine Quint]

Created attachment 432392 [details]
Patch

[Antoine Quint]

<rdar://problem/79863579>

[Tim Horton]

Comment on attachment 432392 [details]
Patch

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

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:74
> +            if ([view isKindOfClass:[WKModelView class]])
> +                return true;

I would expect this to be a WKNativelyInteractible check too?

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:293
> +        if ([view isKindOfClass:[WKModelView class]])
> +            return view;

Why does WKNativelyInteractible not do the trick?

> Source/WebKit/UIProcess/ios/WKModelInteractionGestureRecognizer.mm:60
> +    auto finalTouchesEnded = [touches isEqualToSet:[event touchesForGestureRecognizer:self]];
> +    [self setState:finalTouchesEnded ? UIGestureRecognizerStateEnded : UIGestureRecognizerStateChanged];

What's this weirdness about? (why not just pass the event type along? Are you sure this never means you don't get Ended?

[Antoine Quint]

(In reply to Tim Horton from comment #4)
> Comment on attachment 432392 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432392&action=review
> 
> > Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:74
> > +            if ([view isKindOfClass:[WKModelView class]])
> > +                return true;
> 
> I would expect this to be a WKNativelyInteractible check too?
> 
> > Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:293
> > +        if ([view isKindOfClass:[WKModelView class]])
> > +            return view;
> 
> Why does WKNativelyInteractible not do the trick?

So you suggestI only check for WKNativelyInteractible and not WKModelView?

> > Source/WebKit/UIProcess/ios/WKModelInteractionGestureRecognizer.mm:60
> > +    auto finalTouchesEnded = [touches isEqualToSet:[event touchesForGestureRecognizer:self]];
> > +    [self setState:finalTouchesEnded ? UIGestureRecognizerStateEnded : UIGestureRecognizerStateChanged];
> 
> What's this weirdness about? (why not just pass the event type along? Are
> you sure this never means you don't get Ended?

This is so the gesture recognizer doesn't enter the Ended state until the final touches for this gestures are released. Maybe it's not actually necessary to enter the Ended state and just leave it as Changed?

[Tim Horton]

Comment on attachment 432392 [details]
Patch

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

>>> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:74
>>> +                return true;
>> 
>> I would expect this to be a WKNativelyInteractible check too?
> 
> So you suggestI only check for WKNativelyInteractible and not WKModelView?

Maybe, yes. Certainly better than a WKModelView check.

On the other hand, does this mean that touch-action doesn't work on these? Maybe this is just entirely wrong, and instead you need to get them in the event regions by default?

>>> Source/WebKit/UIProcess/ios/WKModelInteractionGestureRecognizer.mm:60
>>> +    [self setState:finalTouchesEnded ? UIGestureRecognizerStateEnded : UIGestureRecognizerStateChanged];
>> 
>> What's this weirdness about? (why not just pass the event type along? Are you sure this never means you don't get Ended?
> 
> This is so the gesture recognizer doesn't enter the Ended state until the final touches for this gestures are released. Maybe it's not actually necessary to enter the Ended state and just leave it as Changed?

Oh oh oh, I was misreading it. OK, I get it now.

[Antoine Quint]

Created attachment 432478 [details]
Patch

[Tim Horton]

Comment on attachment 432478 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKModelInteractionGestureRecognizer.mm:38
> +- (void)touchesBegan:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event

Likely you could avoid the GR and just use the UIResponder versions of these (on WKModelView directly), but this is also perfectly fine.

[Antoine Quint]

Committed r279402 (239267@main): <https://commits.webkit.org/239267@main>