Web Inspector: Improve UX of Layers tab visualization

• Component: Web Inspector
• Status: RESOLVED FIXED

[Ross Kirsling]

The Layers tab visualization is in need of various UX improvements.

In particular, we need to be able to zoom (restricted by the depth of the current scene) and to move around the scene enough to comfortably see all of the displayed objects (whether that implies pan or something more creative).

Useful test sites:
https://webkit.org/
https://www.playstation.com/en-us/games/horizon-zero-dawn-ps4/
https://www.sony.com/
https://devinrousso.com/projects/

[Ross Kirsling]

Created attachment 325222 [details]
Patch

[Ross Kirsling]

I'd be happy to recount all of the failed experiments on the way to this patch, but for now I'll just summarize the patch contents itself:

0. Rotate: Existing restriction was to allow the camera to rotate in a hemisphere around its target, i.e., it can't go behind the scene and up is always up. This patch further restricts rotation to a horizontal semicircle, as I don't consider vertical rotation to be particularly helpful in this visualization.

1. Zoom: Allow camera distance to vary between a fixed minimum close to the target and a variable maximum based on the depth of the scene (which is currently equivalent to "how many layer objects are being shown").

2. Pan: Allow the camera's target to be any point which is in the scene's bounding box *and is on the XY plane*. (Restricting zoom isn't valid if we start targeting points where Z != 0.) three.js' OrbitControls doesn't support pan restriction so we have to do this from the outside -- this is basically fine but can imply an odd rate of movement depending on the camera rotation.

3. Recentering: On a new document, the camera is repositioned to look at the center of the viewport (i.e. the visible bounds of the document layer) from a distance near to the maximum. When the layer selection is updated from the data grid, the camera is recentered on the selected layer at a "comfortable" (...comfortable unless you've decided to resize WI to be tall and narrow, but you're asking for a suboptimal experience that way anyway).

I've also batched in a couple of uncontroversial fixes here. I can split them out if anybody's unhappy with that.

It goes without saying that any feedback here is welcome, but with this, I've at least reached something I'm not embarrassed to put on display.

[Ross Kirsling]

(In reply to Ross Kirsling from comment #2)
> ...the camera is recentered on the selected layer at a "comfortable" (...).

Forgot the word 'distance' here.

[Ross Kirsling]

Created attachment 325372 [details]
Patch

[Devin Rousso]

Comment on attachment 325372 [details]
Patch

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

r=me.  This feels a LOT better =D

My only negative is that when you select a layer, in addition to moving the camera to center on the layer, it also resets the angle.  I would personally not expect that to happen, and found it annoying, especially on layer-heavy pages where it isn't always obvious how the layers interact.

> Source/WebInspectorUI/UserInterface/Views/LayerDetailsSidebarPanel.js:70
> +        const suppressEvent = true;

NIT: use the actual name `suppressSelectEvent`

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:113
> +        this._controls.minPolarAngle = Math.PI / 2;
> +        this._controls.maxPolarAngle = Math.PI / 2;

I think the user should also be able to pan the Y axis in addition to the X.  If my page is very long (lots of scrolling), I might want to see what the layers look like for items down the page while still scrolled to the top.  It seems oddly restrictive to allow horizontal view changes, but not vertical ones, especially since we often move vertically when changing layer selection.

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:116
> +        this._renderer.domElement.addEventListener("contextmenu", (event) => { event.stopPropagation(); });

Is there a specific reason we want to prevent contextmenu?

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:313
> +        this._controls.target.set(x + width / 2, -y - height / 2, 0);
> +        this._camera.position.set(x + width / 2, -y - height / 2, this._selectedLayerGroup.position.z + WI.Layers3DContentView._zPadding / 2);

Style: we typically wrap different arithmetic operations in parenthesis for clarity.

this._controls.target.set(x + (width / 2), -y - (height / 2), 0);
this._camera.position.set(x + (width / 2), -y - (height / 2), this._selectedLayerGroup.position.z + (WI.Layers3DContentView._zPadding / 2));

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:320
> +        this._controls.target.set(x + width / 2, -y - height / 2, 0);
> +        this._camera.position.set(x + width / 2, -y - height / 2, this._controls.maxDistance - WI.Layers3DContentView._zPadding / 2);

Ditto (312).

[Ross Kirsling]

Created attachment 325637 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 325637 [details]
Patch

Clearing flags on attachment: 325637

Committed r224303: <https://trac.webkit.org/changeset/224303>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35567852>