Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 1 | From 95398982f4a120147a98a27e176facf6d95b2bf7 Mon Sep 17 00:00:00 2001 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 2 | From: Adrian Perez de Castro <aperez@igalia.com> |
| 3 | Date: Thu, 2 Jun 2022 11:19:06 +0300 |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 4 | Subject: [PATCH] FELightningNEON.cpp fails to build, NEON fast path seems |
| 5 | unused https://bugs.webkit.org/show_bug.cgi?id=241182 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 6 | |
| 7 | Reviewed by NOBODY (OOPS!). |
| 8 | |
| 9 | Move the NEON fast path for the SVG lighting filter effects into |
| 10 | FELightingSoftwareApplier, and arrange to actually use them by |
| 11 | forwarding calls to applyPlatformGeneric() into applyPlatformNeon(). |
| 12 | |
| 13 | Some changes were needed to adapt platformApplyNeon() to the current |
| 14 | state of filters after r286140. This was not detected because the code |
| 15 | bitrotted due to it being guarded with CPU(ARM_TRADITIONAL), which does |
| 16 | not get used much these days: CPU(ARM_THUMB2) is more common. It should |
| 17 | be possible to use the NEON fast paths also in Thumb mode, but that is |
| 18 | left for a follow-up fix. |
| 19 | |
| 20 | * Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp: |
| 21 | (WebCore::FELightingSoftwareApplier::platformApplyNeonWorker): |
| 22 | (WebCore::FELightingSoftwareApplier::getPowerCoefficients): |
| 23 | (WebCore::FELighting::platformApplyNeonWorker): Deleted. |
| 24 | (WebCore::FELighting::getPowerCoefficients): Deleted. |
| 25 | * Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h: |
| 26 | (WebCore::FELightingSoftwareApplier::applyPlatformNeon): |
| 27 | (WebCore::FELighting::platformApplyNeon): Deleted. |
| 28 | * Source/WebCore/platform/graphics/filters/DistantLightSource.h: |
| 29 | * Source/WebCore/platform/graphics/filters/FELighting.h: |
| 30 | * Source/WebCore/platform/graphics/filters/PointLightSource.h: |
| 31 | * Source/WebCore/platform/graphics/filters/SpotLightSource.h: |
| 32 | * Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h: |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 33 | |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 34 | Upstream-Status: Submitted [https://github.com/WebKit/WebKit/pull/1233] |
| 35 | Signed-off-by: Khem Raj <raj.khem@gmail.com> |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 36 | |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 37 | --- |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 38 | .../cpu/arm/filters/FELightingNEON.cpp | 4 +- |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 39 | .../graphics/cpu/arm/filters/FELightingNEON.h | 52 +++++++++---------- |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 40 | .../graphics/filters/DistantLightSource.h | 4 ++ |
| 41 | .../platform/graphics/filters/FELighting.h | 7 --- |
| 42 | .../graphics/filters/PointLightSource.h | 4 ++ |
| 43 | .../graphics/filters/SpotLightSource.h | 4 ++ |
| 44 | .../software/FELightingSoftwareApplier.h | 16 ++++++ |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 45 | 7 files changed, 56 insertions(+), 35 deletions(-) |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 46 | |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 47 | diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 48 | index f6ff8c20..42a97ffc 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 49 | --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 50 | +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 51 | @@ -49,7 +49,7 @@ short* feLightingConstantsForNeon() |
| 52 | return s_FELightingConstantsForNeon; |
| 53 | } |
| 54 | |
| 55 | -void FELighting::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| 56 | +void FELightingSoftwareApplier::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| 57 | { |
| 58 | neonDrawLighting(parameters); |
| 59 | } |
| 60 | @@ -464,7 +464,7 @@ TOSTRING(neonDrawLighting) ":" NL |
| 61 | "b .lightStrengthCalculated" NL |
| 62 | ); // NOLINT |
| 63 | |
| 64 | -int FELighting::getPowerCoefficients(float exponent) |
| 65 | +int FELightingSoftwareApplier::getPowerCoefficients(float exponent) |
| 66 | { |
| 67 | // Calling a powf function from the assembly code would require to save |
| 68 | // and reload a lot of NEON registers. Since the base is in range [0..1] |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 69 | diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 70 | index b17c603d..e4629cda 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 71 | --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 72 | +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 73 | @@ -24,14 +24,15 @@ |
| 74 | * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. |
| 75 | */ |
| 76 | |
| 77 | -#ifndef FELightingNEON_h |
| 78 | -#define FELightingNEON_h |
| 79 | +#pragma once |
| 80 | |
| 81 | #if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 82 | |
| 83 | -#include "FELighting.h" |
| 84 | +#include "FELightingSoftwareApplier.h" |
| 85 | +#include "ImageBuffer.h" |
| 86 | #include "PointLightSource.h" |
| 87 | #include "SpotLightSource.h" |
| 88 | +#include <wtf/ObjectIdentifier.h> |
| 89 | #include <wtf/ParallelJobs.h> |
| 90 | |
| 91 | namespace WebCore { |
| 92 | @@ -93,14 +94,14 @@ extern "C" { |
| 93 | void neonDrawLighting(FELightingPaintingDataForNeon*); |
| 94 | } |
| 95 | |
| 96 | -inline void FELighting::platformApplyNeon(const LightingData& data, const LightSource::PaintingData& paintingData) |
| 97 | +inline void FELightingSoftwareApplier::applyPlatformNeon(const FELightingSoftwareApplier::LightingData& data, const LightSource::PaintingData& paintingData) |
| 98 | { |
| 99 | - alignas(16) FELightingFloatArgumentsForNeon floatArguments; |
| 100 | - FELightingPaintingDataForNeon neonData = { |
| 101 | + WebCore::FELightingFloatArgumentsForNeon alignas(16) floatArguments; |
| 102 | + WebCore::FELightingPaintingDataForNeon neonData = { |
| 103 | data.pixels->data(), |
| 104 | 1, |
| 105 | - data.widthDecreasedByOne - 1, |
| 106 | - data.heightDecreasedByOne - 1, |
| 107 | + data.width - 2, |
| 108 | + data.height - 2, |
| 109 | 0, |
| 110 | 0, |
| 111 | 0, |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 112 | @@ -111,23 +112,23 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 113 | // Set light source arguments. |
| 114 | floatArguments.constOne = 1; |
| 115 | |
| 116 | - auto color = m_lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| 117 | + auto color = data.lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| 118 | |
| 119 | floatArguments.colorRed = color.red; |
| 120 | floatArguments.colorGreen = color.green; |
| 121 | floatArguments.colorBlue = color.blue; |
| 122 | floatArguments.padding4 = 0; |
| 123 | |
| 124 | - if (m_lightSource->type() == LS_POINT) { |
| 125 | + if (data.lightSource->type() == LS_POINT) { |
| 126 | neonData.flags |= FLAG_POINT_LIGHT; |
| 127 | - PointLightSource& pointLightSource = static_cast<PointLightSource&>(m_lightSource.get()); |
| 128 | + const auto& pointLightSource = *static_cast<const PointLightSource*>(data.lightSource); |
| 129 | floatArguments.lightX = pointLightSource.position().x(); |
| 130 | floatArguments.lightY = pointLightSource.position().y(); |
| 131 | floatArguments.lightZ = pointLightSource.position().z(); |
| 132 | floatArguments.padding2 = 0; |
| 133 | - } else if (m_lightSource->type() == LS_SPOT) { |
| 134 | + } else if (data.lightSource->type() == LS_SPOT) { |
| 135 | neonData.flags |= FLAG_SPOT_LIGHT; |
| 136 | - SpotLightSource& spotLightSource = static_cast<SpotLightSource&>(m_lightSource.get()); |
| 137 | + const auto& spotLightSource = *static_cast<const SpotLightSource*>(data.lightSource); |
| 138 | floatArguments.lightX = spotLightSource.position().x(); |
| 139 | floatArguments.lightY = spotLightSource.position().y(); |
| 140 | floatArguments.lightZ = spotLightSource.position().z(); |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 141 | @@ -145,7 +146,7 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 142 | if (spotLightSource.specularExponent() == 1) |
| 143 | neonData.flags |= FLAG_CONE_EXPONENT_IS_1; |
| 144 | } else { |
| 145 | - ASSERT(m_lightSource->type() == LS_DISTANT); |
| 146 | + ASSERT(data.lightSource->type() == LS_DISTANT); |
| 147 | floatArguments.lightX = paintingData.initialLightingData.lightVector.x(); |
| 148 | floatArguments.lightY = paintingData.initialLightingData.lightVector.y(); |
| 149 | floatArguments.lightZ = paintingData.initialLightingData.lightVector.z(); |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 150 | @@ -155,38 +156,39 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 151 | // Set lighting arguments. |
| 152 | floatArguments.surfaceScale = data.surfaceScale; |
| 153 | floatArguments.minusSurfaceScaleDividedByFour = -data.surfaceScale / 4; |
| 154 | - if (m_lightingType == FELighting::DiffuseLighting) |
| 155 | - floatArguments.diffuseConstant = m_diffuseConstant; |
| 156 | + if (data.filterType == FilterEffect::Type::FEDiffuseLighting) |
| 157 | + floatArguments.diffuseConstant = data.diffuseConstant; |
| 158 | else { |
| 159 | neonData.flags |= FLAG_SPECULAR_LIGHT; |
| 160 | - floatArguments.diffuseConstant = m_specularConstant; |
| 161 | - neonData.specularExponent = getPowerCoefficients(m_specularExponent); |
| 162 | - if (m_specularExponent == 1) |
| 163 | + floatArguments.diffuseConstant = data.specularConstant; |
| 164 | + neonData.specularExponent = getPowerCoefficients(data.specularExponent); |
| 165 | + if (data.specularExponent == 1) |
| 166 | neonData.flags |= FLAG_SPECULAR_EXPONENT_IS_1; |
| 167 | } |
| 168 | if (floatArguments.diffuseConstant == 1) |
| 169 | neonData.flags |= FLAG_DIFFUSE_CONST_IS_1; |
| 170 | |
| 171 | - int optimalThreadNumber = ((data.widthDecreasedByOne - 1) * (data.heightDecreasedByOne - 1)) / s_minimalRectDimension; |
| 172 | + static constexpr int minimalRectDimension = 100 * 100; // Empirical data limit for parallel jobs |
| 173 | + int optimalThreadNumber = ((data.width - 2) * (data.height - 2)) / minimalRectDimension; |
| 174 | if (optimalThreadNumber > 1) { |
| 175 | // Initialize parallel jobs |
| 176 | - ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&WebCore::FELighting::platformApplyNeonWorker, optimalThreadNumber); |
| 177 | + ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&FELightingSoftwareApplier::platformApplyNeonWorker, optimalThreadNumber); |
| 178 | |
| 179 | // Fill the parameter array |
| 180 | int job = parallelJobs.numberOfJobs(); |
| 181 | if (job > 1) { |
| 182 | int yStart = 1; |
| 183 | - int yStep = (data.heightDecreasedByOne - 1) / job; |
| 184 | + int yStep = (data.height - 2) / job; |
| 185 | for (--job; job >= 0; --job) { |
| 186 | FELightingPaintingDataForNeon& params = parallelJobs.parameter(job); |
| 187 | params = neonData; |
| 188 | params.yStart = yStart; |
| 189 | - params.pixels += (yStart - 1) * (data.widthDecreasedByOne + 1) * 4; |
| 190 | + params.pixels += (yStart - 1) * data.width * 4; |
| 191 | if (job > 0) { |
| 192 | params.absoluteHeight = yStep; |
| 193 | yStart += yStep; |
| 194 | } else |
| 195 | - params.absoluteHeight = data.heightDecreasedByOne - yStart; |
| 196 | + params.absoluteHeight = (data.height - 1) - yStart; |
| 197 | } |
| 198 | parallelJobs.execute(); |
| 199 | return; |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 200 | @@ -199,5 +201,3 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 201 | } // namespace WebCore |
| 202 | |
| 203 | #endif // CPU(ARM_NEON) && COMPILER(GCC_COMPATIBLE) |
| 204 | - |
| 205 | -#endif // FELightingNEON_h |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 206 | diff --git a/Source/WebCore/platform/graphics/filters/DistantLightSource.h b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 207 | index 70f583b3..7d5d27e5 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 208 | --- a/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| 209 | +++ b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 210 | @@ -26,6 +26,10 @@ |
| 211 | #include <wtf/ArgumentCoder.h> |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 212 | #include <wtf/Ref.h> |
| 213 | |
| 214 | +namespace WTF { |
| 215 | +class TextStream; |
| 216 | +} // namespace WTF |
| 217 | + |
| 218 | namespace WebCore { |
| 219 | |
| 220 | class DistantLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 221 | diff --git a/Source/WebCore/platform/graphics/filters/FELighting.h b/Source/WebCore/platform/graphics/filters/FELighting.h |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 222 | index 179edf6d..694d712d 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 223 | --- a/Source/WebCore/platform/graphics/filters/FELighting.h |
| 224 | +++ b/Source/WebCore/platform/graphics/filters/FELighting.h |
| 225 | @@ -35,8 +35,6 @@ |
| 226 | |
| 227 | namespace WebCore { |
| 228 | |
| 229 | -struct FELightingPaintingDataForNeon; |
| 230 | - |
| 231 | class FELighting : public FilterEffect { |
| 232 | public: |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 233 | bool operator==(const FELighting&) const; |
| 234 | @@ -68,11 +66,6 @@ protected: |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 235 | |
| 236 | std::unique_ptr<FilterEffectApplier> createSoftwareApplier() const override; |
| 237 | |
| 238 | -#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 239 | - static int getPowerCoefficients(float exponent); |
| 240 | - inline void platformApplyNeon(const LightingData&, const LightSource::PaintingData&); |
| 241 | -#endif |
| 242 | - |
| 243 | Color m_lightingColor; |
| 244 | float m_surfaceScale; |
| 245 | float m_diffuseConstant; |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 246 | diff --git a/Source/WebCore/platform/graphics/filters/PointLightSource.h b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 247 | index a8cfdab8..34f867bb 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 248 | --- a/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| 249 | +++ b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| 250 | @@ -26,6 +26,10 @@ |
| 251 | #include "LightSource.h" |
| 252 | #include <wtf/Ref.h> |
| 253 | |
| 254 | +namespace WTF { |
| 255 | +class TextStream; |
| 256 | +} // namespace WTF |
| 257 | + |
| 258 | namespace WebCore { |
| 259 | |
| 260 | class PointLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 261 | diff --git a/Source/WebCore/platform/graphics/filters/SpotLightSource.h b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
Patrick Williams | da29531 | 2023-12-05 16:48:56 -0600 | [diff] [blame] | 262 | index 6404467a..5cac38f2 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 263 | --- a/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| 264 | +++ b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| 265 | @@ -26,6 +26,10 @@ |
| 266 | #include "LightSource.h" |
| 267 | #include <wtf/Ref.h> |
| 268 | |
| 269 | +namespace WTF { |
| 270 | +class TextStream; |
| 271 | +} // namespace WTF |
| 272 | + |
| 273 | namespace WebCore { |
| 274 | |
| 275 | class SpotLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 276 | diff --git a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 277 | index c974d921..e2896660 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 278 | --- a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 279 | +++ b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 280 | @@ -36,6 +36,7 @@ |
| 281 | namespace WebCore { |
| 282 | |
| 283 | class FELighting; |
| 284 | +struct FELightingPaintingDataForNeon; |
| 285 | |
| 286 | class FELightingSoftwareApplier final : public FilterEffectConcreteApplier<FELighting> { |
| 287 | WTF_MAKE_FAST_ALLOCATED; |
| 288 | @@ -132,8 +133,23 @@ private: |
| 289 | |
| 290 | static void applyPlatformGenericPaint(const LightingData&, const LightSource::PaintingData&, int startY, int endY); |
| 291 | static void applyPlatformGenericWorker(ApplyParameters*); |
| 292 | + |
| 293 | +#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 294 | + static int getPowerCoefficients(float exponent); |
| 295 | + static void platformApplyNeonWorker(FELightingPaintingDataForNeon*); |
| 296 | + inline static void applyPlatformNeon(const LightingData&, const LightSource::PaintingData&); |
| 297 | + |
| 298 | + inline static void applyPlatformGeneric(const LightingData& data, const LightSource::PaintingData& paintingData) |
| 299 | + { |
| 300 | + applyPlatformNeon(data, paintingData); |
| 301 | + } |
| 302 | +#else |
| 303 | static void applyPlatformGeneric(const LightingData&, const LightSource::PaintingData&); |
| 304 | +#endif |
| 305 | + |
| 306 | static void applyPlatform(const LightingData&); |
| 307 | }; |
| 308 | |
| 309 | } // namespace WebCore |
| 310 | + |
| 311 | +#include "FELightingNEON.h" |