Created attachment 444038 [details] test case Web Inspector: Layer Tab: the position of composited layer with box-shadow is wrong 1. Open the test case 2. Open Web Inspector 3. Open Layer tab 4. Select canvas element 5. the position of the composited layer is wrong
Created attachment 444039 [details] wrong screenshot The orange layer of <canvas> is placed correctly on the webview. But, the blue box layer of <canvas> in the layers tab is wrongly placed at the visible area of it.
Comment on attachment 444039 [details] wrong screenshot My bad. This screenshot is wrong. the orange are is just a margin area. It's not a layer.
Created attachment 444041 [details] screenshot Mac MiniBrowser can show Layer Border. Because this <canvas> has a large box-shadow, it has a large layer. Layers Tab shows <canvas> layer box placed wrongly at the visible position of the element.
Created attachment 444044 [details] Patch
Comment on attachment 444044 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444044&action=review > Source/WebInspectorUI/ChangeLog:8 > + If an elemenet has box-shadow, Layers Tab showed the composited Nit: element
Comment on attachment 444044 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444044&action=review r- as this doesn't take into account older inspected targets, but the logic looks good otherwise :) > Source/WebInspectorUI/UserInterface/Models/Layer.js:-46 > - // FIXME: This should probably be moved to the backend. > - this._compositedBounds.x = this._bounds.x; > - this._compositedBounds.y = this._bounds.y; This is not a valid fix when remotely inspecting a backend that does _not_ have the above change (e.g. inspecting an older iOS). You'll need to add some sort of compatibility check. AFAICT, the only thing that's changed in the protocol that's checkable is `InspectorBackend.Enum.CSS.PseudoId.Backdrop` (from r281229), so you'd wanna do something like this ``` // COMPATIBILITY (iOS 15.0): the `bounds` and `computedBounds` properties of `LayerTree.Layer` did not account for parent element positioning. // Since support can't be tested directly, check for the `Backdrop` value in the `CSS.PseudoId` enum. // FIXME: Use explicit version checking once <https://webkit.org/b/148680> is fixed. if (!InspectorBackend.Enum.CSS.PseudoId.Backdrop) { this._compositedBounds.x = this._bounds.x; this._compositedBounds.y = this._bounds.y; } ``` > Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:272 > - layerGroup.position.set(layer.bounds.x, -layer.bounds.y, index * zInterval); > + layerGroup.position.set(0, 0, index * zInterval); ditto (Source/WebInspectorUI/UserInterface/Models/Layer.js:44) > Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:289 > + _createLayerMesh({x, y, width, height}, isOutline = false) ditto (Source/WebInspectorUI/UserInterface/Models/Layer.js:44) tho in this case it may be clearer to adjust the callsite instead 🤔
Comment on attachment 444044 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444044&action=review >> Source/WebInspectorUI/UserInterface/Models/Layer.js:-46 >> - this._compositedBounds.y = this._bounds.y; > > This is not a valid fix when remotely inspecting a backend that does _not_ have the above change (e.g. inspecting an older iOS). You'll need to add some sort of compatibility check. AFAICT, the only thing that's changed in the protocol that's checkable is `InspectorBackend.Enum.CSS.PseudoId.Backdrop` (from r281229), so you'd wanna do something like this > ``` > // COMPATIBILITY (iOS 15.0): the `bounds` and `computedBounds` properties of `LayerTree.Layer` did not account for parent element positioning. > // Since support can't be tested directly, check for the `Backdrop` value in the `CSS.PseudoId` enum. > // FIXME: Use explicit version checking once <https://webkit.org/b/148680> is fixed. > if (!InspectorBackend.Enum.CSS.PseudoId.Backdrop) { > this._compositedBounds.x = this._bounds.x; > this._compositedBounds.y = this._bounds.y; > } > ``` Good point. This issue can be fixed without the protocol change. I think it's better. Will fix.
Created attachment 444184 [details] Patch
Comment on attachment 444184 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444184&action=review r=me, nice! > Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:296 > + new THREE.Vector3(x, -y-height, 0), > + new THREE.Vector3(x+width, -y-height, 0), > + new THREE.Vector3(x+width, -y, 0), Style: please add spaces between operators ``` new THREE.Vector3(x, -y - height, 0), new THREE.Vector3(x + width, -y - height, 0), new THREE.Vector3(x + width, -y, 0), ```
Created attachment 444302 [details] Patch for landing Thank you for the review.
Committed r285839 (244271@main): <https://commits.webkit.org/244271@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444302 [details].
<rdar://problem/85433224>