[Blink Geometry Mapper Transfrom] Add fast path for z-axis translation [chromium/src : main]
Jayson Adams (Gerrit)
Attention is currently required from: Stephen Chenney.
Jayson Adams would like Stephen Chenney to review this change.
[Blink Geometry Mapper Transfrom] Add fast path for z-axis translation
UpdateScreenTransform() in the GeometryMapperTransformCache has a fast
path for unity transforms or simple 2d translations. All other
transforms go through the slow path which consists of 64
mutliplications and 48 additions. However, there's another fast path
case: a simple z-axis transformation, which requires only 4 multiplies
and 4 adds. This cl adds a fast path for the simple z-axis transform
case.
Bug: 1336059
Test: out/Default/blink_platform_unittests \
--gtest_filter=TransformationMatrixTest*
Change-Id: I9e35e6df17bdaef37e4b946daa658a3d99b376d0
---
M third_party/blink/renderer/platform/graphics/paint/geometry_mapper_transform_cache.cc
M third_party/blink/renderer/platform/transforms/transformation_matrix.cc
M third_party/blink/renderer/platform/transforms/transformation_matrix.h
M third_party/blink/renderer/platform/transforms/transformation_matrix_test.cc
4 files changed, 112 insertions(+), 0 deletions(-)
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Jayson Adams (Gerrit)
Attention is currently required from: Stephen Chenney.
1 comment:
Patchset:
Hello schenney@, PTAL.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
Attention is currently required from: Stephen Chenney, Xianzhu Wang.
Philip Rogers would like Xianzhu Wang to review this change authored by Jayson Adams.
[Blink Geometry Mapper Transfrom] Add fast path for z-axis translation
UpdateScreenTransform() in the GeometryMapperTransformCache has a fast
path for unity transforms or simple 2d translations. All other
transforms go through the slow path which consists of 64
mutliplications and 48 additions. However, there's another fast path
case: a simple z-axis transformation, which requires only 4 multiplies
and 4 adds. This cl adds a fast path for the simple z-axis transform
case.
Bug: 1336059
Test: out/Default/blink_platform_unittests \
--gtest_filter=TransformationMatrixTest*
Change-Id: I9e35e6df17bdaef37e4b946daa658a3d99b376d0
---
M third_party/blink/renderer/platform/graphics/paint/geometry_mapper_transform_cache.cc
M third_party/blink/renderer/platform/transforms/transformation_matrix.cc
M third_party/blink/renderer/platform/transforms/transformation_matrix.h
M third_party/blink/renderer/platform/transforms/transformation_matrix_test.cc
4 files changed, 112 insertions(+), 0 deletions(-)
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Attention is currently required from: Jayson Adams, Stephen Chenney.
1 comment:
Patchset:
A data point from Skia: They simplified SkMatrix44 to SkM44 to just use the "slow" path for all matrix operations and they observed performance improvements. Their theory is that the reduced branch instructions makes the pipeline and SIMD operations more efficient. I'm not sure their theory applies to blink. I planned to investigate this, but for now I'm not actively working on it.
Jayson, do you have performance test data for this patch?
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Patchset:
A data point from Skia: They simplified SkMatrix44 to SkM44 to just use the "slow" path for all matr […]
I saw the improvement described in the bug. I would like to see the impact of this CL on generic 3d transform benchmarks because the added condition may have a negative performance impact.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Patchset:
I saw the improvement described in the bug. […]
How about a different method: assuming PendingLayers with transforms that are not in the same plane as overlapping? In this way we'll skip overlap testing for these PendingLayers.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Jayson Adams (Gerrit)
Attention is currently required from: Stephen Chenney, Xianzhu Wang.
1 comment:
Patchset:
How about a different method: assuming PendingLayers with transforms that are not in the same plane […]
Hello wangxianzhu@, thanks for taking a look! It sounds like you're saying we should investigate the approach you suggest rather than the change I propose in my cl? That's fine, but I am unable to code that up myself (at least not without a lot of pointers). I know little about blink and layers and 3d transforms.
I'm also happy to run this change against the generic 3d transform benchmarks - I just need a pointer to doing that.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Attention is currently required from: Jayson Adams, Stephen Chenney.
1 comment:
Patchset:
Hello wangxianzhu@, thanks for taking a look! It sounds like you're saying we should investigate the […]
The approach will require the following changes:
1. Extract a function SourceToDestinationProjectionInternalSamePlaneOnly() from GeometryMapper::SourceToDestinationProjectionInternal(), to include the part before Case 2. In the function, set success = false before returning. The latter can be changed to call the new function before mapping in the screen space.
2. In LocalToAncestorVisualRectInternal(), if expand==kExpandVisualRectForCompositingOverlap, call the new function instead of the old function, and if success is false, return InfiniteLooseFloatClipRect().Can you introduce some background about the bug and the CL, e.g. how they are related to your work, so that we can decide how much we can ask you to do, etc.?
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Jayson Adams (Gerrit)
Attention is currently required from: Stephen Chenney, Xianzhu Wang.
1 comment:
Patchset:
The approach will require the following changes: […]
Hello,
Sorry I've been out of touch for so long. We went into a long weekend and then vacation for me, then prep for a week-long conference in NYC and the conference itself. Anyways, I'm back now.
A couple questions:
- You said SourceToDestinationProjectionInternalSamePlaneOnly() should include the part before Case 2. Do you mean everything before Case 2, or just the code for Case 1a and 1b?
- You say set success = false before returning. Do you mean in all cases? That seems odd if so, but I don't know when it would be set to true.
Re: background on the bug and CL, I work on investigating excess CPU use in Chrome Mac. I was profiling the renderer, saw this case, and took a stab at a fix. I'm up for making changes to the code to improve performance but I'm not a graphics expert (or web platform person either), so I need some help making heads or tails of things.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Attention is currently required from: Jayson Adams, Stephen Chenney.
1 comment:
Patchset:
Hello, […]
Now we are investigating reducing PendingLayer::MightOverlap() invokes (partly inspired by your work, thanks!), and a WIP CL https://chromium-review.googlesource.com/c/chromium/src/+/3788768 may help crbug.com/1336059. Can you try the CL? Particularly we want to know how many MightOverlap() invokes are eliminated for the bug.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Jayson Adams (Gerrit)
Jayson Adams abandoned this change.
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.
Jayson Adams (Gerrit)
1 comment:
Patchset:
Now we are investigating reducing PendingLayer::MightOverlap() invokes (partly inspired by your work […]
This bug is tricky to repro (it depends on the ads the site is serving). Comparing Chrome with and without your CL (no longer WIP?), the percentage of main thread time spent in blink::PendingLayer::MightOverlap() drops by 28% with the CL applied. I can also see that the number of blink::TransformationMatrix::Multiply()s drops to almost nothing. Thanks for making the change!!
To view, visit change 3704397. To unsubscribe, or for help writing mail filters, visit settings.