Skip to content

Regression(280901@main)? [WPE][TextureMapper] Incorrect extra hole punches with fixed body position#36120

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
magomez:eng/Regression280901main-WPETextureMapper-Incorrect-extra-hole-punches-with-fixed-body-position
Nov 5, 2024
Merged

Regression(280901@main)? [WPE][TextureMapper] Incorrect extra hole punches with fixed body position#36120
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
magomez:eng/Regression280901main-WPETextureMapper-Incorrect-extra-hole-punches-with-fixed-body-position

Conversation

@magomez
Copy link
Copy Markdown
Contributor

@magomez magomez commented Nov 4, 2024

4df9391

Regression(280901@main)? [WPE][TextureMapper] Incorrect extra hole punches with fixed body position
https://bugs.webkit.org/show_bug.cgi?id=281309

Reviewed by Carlos Garcia Campos.

When rendering into a preserves3D subtree, we're rendering into an intermediate surface
that, once ready, is blended into the main framebuffer.
To make holepunching work in this situation we need to render the transparent rectangle
in the intermediate surface, but also in the main framebuffer. To achieve this, when
rendering into the intermediate surface we keep a vector of rects that need to be painted
to the main framebuffer just before blending the intermediate surface.
There are a couple of points that need to be taken into account though: the intermediate
surface rendering is tiled, so the same TextureMapperLayer can be rendered several times,
and it will receive an offset to compensate the rendering into the intermediate surface.

In order to notify the video sink and render the transparent rectange in the main
framebuffer, we need to use a transform that doesn't include the intermediate surface
offset. But to paint into the intermediate surface we need to use the offsetted transform.
To achieve this, we calculate the non offsetted transform in the first tile, and use it
to notify the real video position to the video sink and the main framebuffer. Then
we use the normal transform to paint the transparent rectangle into the intermediate
surface.

* Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:
(WebCore::TextureMapperLayer::paintSelf):
(WebCore::TextureMapperLayer::paintPreserves3DHolePunch):
(WebCore::TextureMapperLayer::paintWith3DRenderingContext):
* Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:
* Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:
(WebCore::TextureMapperPlatformLayer::notifyVideoPosition):
(WebCore::TextureMapperPlatformLayer::paintTransparentRectangle):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferHolePunch.cpp:
(WebCore::CoordinatedPlatformLayerBufferHolePunch::notifyVideoPosition):
(WebCore::CoordinatedPlatformLayerBufferHolePunch::paintTransparentRectangle):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferHolePunch.h:

Canonical link: https://commits.webkit.org/286146@main

076331c

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@magomez magomez requested a review from zdobersek as a code owner November 4, 2024 11:07
@magomez magomez self-assigned this Nov 4, 2024
@magomez magomez added the WPE WebKit WebKit WPE component label Nov 4, 2024
@magomez magomez requested a review from carlosgcampos November 4, 2024 11:18
Comment thread Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h Outdated
@fujii
Copy link
Copy Markdown
Contributor

fujii commented Nov 4, 2024

an intermediate surface is used not only by preserve 3d layers.
hole punch should be processed in three steps.

  1. Paint layers prio to the hole punch layer
  2. paint a transparent rect
  3. Paint layers after the hole punch layer

@magomez
Copy link
Copy Markdown
Contributor Author

magomez commented Nov 4, 2024

an intermediate surface is used not only by preserve 3d layers. hole punch should be processed in three steps.

Yes, I know. But I'm not trying to fix the general case here. Keeping the holepunch working for every possible situation is very complex. the goal is to keep it working in common scenarios. These common scenarios are: rendering to the main framebuffer (which is already working) and rendering to inside an stacking context that was created because of the position:fixed added to the body element.

  1. Paint layers prio to the hole punch layer

Sure, that's the normal behavior.

  1. paint a transparent rect

This is the tricky part. When using an intermediate surface, it's not as simple as painting a transparent rect. We need to paint the rect in the main framebuffer and also in the intermediate surface, in a way that they overlap each other and the position where the video will be put below. This is because the intermediate texture is blended into the main frambuffer, so even if we we have a transparent rectangle there, it won't pierce the background.
So what I do is paint the transparent rectangle into the intermediate texture, and and save the rect, so just before blending the intermediate texture, I paint a transparent rectangle in the main framebuffer.

  1. Paint layers after the hole punch layer

Normal behavior again.

@fujii
Copy link
Copy Markdown
Contributor

fujii commented Nov 4, 2024

I mean paiting the transparent rect outside of paintRecursive.
A psuedo code looks like:

paintUntilHolePunchLayer();
paintHolePunchLayer();
paintAfterHolePunchLayer();

Then, we don't need to paint a transparent rect to intermediate buffers.

@magomez
Copy link
Copy Markdown
Contributor Author

magomez commented Nov 5, 2024

I mean paiting the transparent rect outside of paintRecursive. A psuedo code looks like:

paintUntilHolePunchLayer();
paintHolePunchLayer();
paintAfterHolePunchLayer();

Then, we don't need to paint a transparent rect to intermediate buffers.

I'm afraid I don't follow. Where does that pseudocode fit in the current painting code? Do you mean adding a second traverse of the tree to paint just the holepuch, besides the current paintRecursive? In that case, the hole will be painted before or after the normal content, but not in the appropriate order (defined by the position of the layer). Are you assuming that the holepunch can be painted after paintRecursive? Cause that's not the case, as overlays can be put over the transparent rect.
Maybe I'm too focused on my approach, but I fail to see how it can be implemented without painting into the intermediate textures.

@magomez magomez added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing GLib Suggested Backport - 2.46 labels Nov 5, 2024
…nches with fixed body position

https://bugs.webkit.org/show_bug.cgi?id=281309

Reviewed by Carlos Garcia Campos.

When rendering into a preserves3D subtree, we're rendering into an intermediate surface
that, once ready, is blended into the main framebuffer.
To make holepunching work in this situation we need to render the transparent rectangle
in the intermediate surface, but also in the main framebuffer. To achieve this, when
rendering into the intermediate surface we keep a vector of rects that need to be painted
to the main framebuffer just before blending the intermediate surface.
There are a couple of points that need to be taken into account though: the intermediate
surface rendering is tiled, so the same TextureMapperLayer can be rendered several times,
and it will receive an offset to compensate the rendering into the intermediate surface.

In order to notify the video sink and render the transparent rectange in the main
framebuffer, we need to use a transform that doesn't include the intermediate surface
offset. But to paint into the intermediate surface we need to use the offsetted transform.
To achieve this, we calculate the non offsetted transform in the first tile, and use it
to notify the real video position to the video sink and the main framebuffer. Then
we use the normal transform to paint the transparent rectangle into the intermediate
surface.

* Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:
(WebCore::TextureMapperLayer::paintSelf):
(WebCore::TextureMapperLayer::paintPreserves3DHolePunch):
(WebCore::TextureMapperLayer::paintWith3DRenderingContext):
* Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:
* Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:
(WebCore::TextureMapperPlatformLayer::notifyVideoPosition):
(WebCore::TextureMapperPlatformLayer::paintTransparentRectangle):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferHolePunch.cpp:
(WebCore::CoordinatedPlatformLayerBufferHolePunch::notifyVideoPosition):
(WebCore::CoordinatedPlatformLayerBufferHolePunch::paintTransparentRectangle):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayerBufferHolePunch.h:

Canonical link: https://commits.webkit.org/286146@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Regression280901main-WPETextureMapper-Incorrect-extra-hole-punches-with-fixed-body-position branch from 076331c to 4df9391 Compare November 5, 2024 13:14
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 286146@main (4df9391): https://commits.webkit.org/286146@main

Reviewed commits have been landed. Closing PR #36120 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 4df9391 into WebKit:main Nov 5, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 5, 2024
@fujii
Copy link
Copy Markdown
Contributor

fujii commented Nov 5, 2024

I'm afraid I don't follow.
Where does that pseudocode fit in the current painting code?

For example, TextureMapperLayer::paint.

Do you mean adding a second traverse of the tree to paint just the holepuch, besides the current paintRecursive?

paintHolePunchLayer() doesn't traverse the layer tree.

In that case, the hole will be painted before or after the normal content, but not in the appropriate order (defined by the position of the layer).

Hope punching doesn't work well with filter and opacity, etc.

Are you assuming that the holepunch can be painted after paintRecursive?

paintAfterHolePunchLayer() renders layers after the hole.

Cause that's not the case, as overlays can be put over the transparent rect.

paintAfterHolePunchLayer() renders layers over the hole.

Maybe I'm too focused on my approach, but I fail to see how it can be implemented without painting into the intermediate textures.

I have a plan to rewrite some day in the future.

@aperezdc
Copy link
Copy Markdown
Contributor

Backported into the 2.46 branch as commit f5716b1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WPE WebKit WebKit WPE component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants