Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 1 | From 647c93de99a0f71f478d76a4cc7714eba7ba1447 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> |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 36 | --- |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 37 | .../cpu/arm/filters/FELightingNEON.cpp | 4 +- |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 38 | .../graphics/cpu/arm/filters/FELightingNEON.h | 52 +++++++++---------- |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 39 | .../graphics/filters/DistantLightSource.h | 4 ++ |
| 40 | .../platform/graphics/filters/FELighting.h | 7 --- |
| 41 | .../graphics/filters/PointLightSource.h | 4 ++ |
| 42 | .../graphics/filters/SpotLightSource.h | 4 ++ |
| 43 | .../software/FELightingSoftwareApplier.h | 16 ++++++ |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 44 | 7 files changed, 56 insertions(+), 35 deletions(-) |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 45 | |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 46 | diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 47 | index f6ff8c20..42a97ffc 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 48 | --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 49 | +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| 50 | @@ -49,7 +49,7 @@ short* feLightingConstantsForNeon() |
| 51 | return s_FELightingConstantsForNeon; |
| 52 | } |
| 53 | |
| 54 | -void FELighting::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| 55 | +void FELightingSoftwareApplier::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| 56 | { |
| 57 | neonDrawLighting(parameters); |
| 58 | } |
| 59 | @@ -464,7 +464,7 @@ TOSTRING(neonDrawLighting) ":" NL |
| 60 | "b .lightStrengthCalculated" NL |
| 61 | ); // NOLINT |
| 62 | |
| 63 | -int FELighting::getPowerCoefficients(float exponent) |
| 64 | +int FELightingSoftwareApplier::getPowerCoefficients(float exponent) |
| 65 | { |
| 66 | // Calling a powf function from the assembly code would require to save |
| 67 | // 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] | 68 | diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 69 | index b17c603d..e4629cda 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 70 | --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 71 | +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| 72 | @@ -24,14 +24,15 @@ |
| 73 | * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. |
| 74 | */ |
| 75 | |
| 76 | -#ifndef FELightingNEON_h |
| 77 | -#define FELightingNEON_h |
| 78 | +#pragma once |
| 79 | |
| 80 | #if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 81 | |
| 82 | -#include "FELighting.h" |
| 83 | +#include "FELightingSoftwareApplier.h" |
| 84 | +#include "ImageBuffer.h" |
| 85 | #include "PointLightSource.h" |
| 86 | #include "SpotLightSource.h" |
| 87 | +#include <wtf/ObjectIdentifier.h> |
| 88 | #include <wtf/ParallelJobs.h> |
| 89 | |
| 90 | namespace WebCore { |
| 91 | @@ -93,14 +94,14 @@ extern "C" { |
| 92 | void neonDrawLighting(FELightingPaintingDataForNeon*); |
| 93 | } |
| 94 | |
| 95 | -inline void FELighting::platformApplyNeon(const LightingData& data, const LightSource::PaintingData& paintingData) |
| 96 | +inline void FELightingSoftwareApplier::applyPlatformNeon(const FELightingSoftwareApplier::LightingData& data, const LightSource::PaintingData& paintingData) |
| 97 | { |
| 98 | - alignas(16) FELightingFloatArgumentsForNeon floatArguments; |
| 99 | - FELightingPaintingDataForNeon neonData = { |
| 100 | + WebCore::FELightingFloatArgumentsForNeon alignas(16) floatArguments; |
| 101 | + WebCore::FELightingPaintingDataForNeon neonData = { |
| 102 | data.pixels->data(), |
| 103 | 1, |
| 104 | - data.widthDecreasedByOne - 1, |
| 105 | - data.heightDecreasedByOne - 1, |
| 106 | + data.width - 2, |
| 107 | + data.height - 2, |
| 108 | 0, |
| 109 | 0, |
| 110 | 0, |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 111 | @@ -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] | 112 | // Set light source arguments. |
| 113 | floatArguments.constOne = 1; |
| 114 | |
| 115 | - auto color = m_lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| 116 | + auto color = data.lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| 117 | |
| 118 | floatArguments.colorRed = color.red; |
| 119 | floatArguments.colorGreen = color.green; |
| 120 | floatArguments.colorBlue = color.blue; |
| 121 | floatArguments.padding4 = 0; |
| 122 | |
| 123 | - if (m_lightSource->type() == LS_POINT) { |
| 124 | + if (data.lightSource->type() == LS_POINT) { |
| 125 | neonData.flags |= FLAG_POINT_LIGHT; |
| 126 | - PointLightSource& pointLightSource = static_cast<PointLightSource&>(m_lightSource.get()); |
| 127 | + const auto& pointLightSource = *static_cast<const PointLightSource*>(data.lightSource); |
| 128 | floatArguments.lightX = pointLightSource.position().x(); |
| 129 | floatArguments.lightY = pointLightSource.position().y(); |
| 130 | floatArguments.lightZ = pointLightSource.position().z(); |
| 131 | floatArguments.padding2 = 0; |
| 132 | - } else if (m_lightSource->type() == LS_SPOT) { |
| 133 | + } else if (data.lightSource->type() == LS_SPOT) { |
| 134 | neonData.flags |= FLAG_SPOT_LIGHT; |
| 135 | - SpotLightSource& spotLightSource = static_cast<SpotLightSource&>(m_lightSource.get()); |
| 136 | + const auto& spotLightSource = *static_cast<const SpotLightSource*>(data.lightSource); |
| 137 | floatArguments.lightX = spotLightSource.position().x(); |
| 138 | floatArguments.lightY = spotLightSource.position().y(); |
| 139 | floatArguments.lightZ = spotLightSource.position().z(); |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 140 | @@ -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] | 141 | if (spotLightSource.specularExponent() == 1) |
| 142 | neonData.flags |= FLAG_CONE_EXPONENT_IS_1; |
| 143 | } else { |
| 144 | - ASSERT(m_lightSource->type() == LS_DISTANT); |
| 145 | + ASSERT(data.lightSource->type() == LS_DISTANT); |
| 146 | floatArguments.lightX = paintingData.initialLightingData.lightVector.x(); |
| 147 | floatArguments.lightY = paintingData.initialLightingData.lightVector.y(); |
| 148 | floatArguments.lightZ = paintingData.initialLightingData.lightVector.z(); |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 149 | @@ -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] | 150 | // Set lighting arguments. |
| 151 | floatArguments.surfaceScale = data.surfaceScale; |
| 152 | floatArguments.minusSurfaceScaleDividedByFour = -data.surfaceScale / 4; |
| 153 | - if (m_lightingType == FELighting::DiffuseLighting) |
| 154 | - floatArguments.diffuseConstant = m_diffuseConstant; |
| 155 | + if (data.filterType == FilterEffect::Type::FEDiffuseLighting) |
| 156 | + floatArguments.diffuseConstant = data.diffuseConstant; |
| 157 | else { |
| 158 | neonData.flags |= FLAG_SPECULAR_LIGHT; |
| 159 | - floatArguments.diffuseConstant = m_specularConstant; |
| 160 | - neonData.specularExponent = getPowerCoefficients(m_specularExponent); |
| 161 | - if (m_specularExponent == 1) |
| 162 | + floatArguments.diffuseConstant = data.specularConstant; |
| 163 | + neonData.specularExponent = getPowerCoefficients(data.specularExponent); |
| 164 | + if (data.specularExponent == 1) |
| 165 | neonData.flags |= FLAG_SPECULAR_EXPONENT_IS_1; |
| 166 | } |
| 167 | if (floatArguments.diffuseConstant == 1) |
| 168 | neonData.flags |= FLAG_DIFFUSE_CONST_IS_1; |
| 169 | |
| 170 | - int optimalThreadNumber = ((data.widthDecreasedByOne - 1) * (data.heightDecreasedByOne - 1)) / s_minimalRectDimension; |
| 171 | + static constexpr int minimalRectDimension = 100 * 100; // Empirical data limit for parallel jobs |
| 172 | + int optimalThreadNumber = ((data.width - 2) * (data.height - 2)) / minimalRectDimension; |
| 173 | if (optimalThreadNumber > 1) { |
| 174 | // Initialize parallel jobs |
| 175 | - ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&WebCore::FELighting::platformApplyNeonWorker, optimalThreadNumber); |
| 176 | + ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&FELightingSoftwareApplier::platformApplyNeonWorker, optimalThreadNumber); |
| 177 | |
| 178 | // Fill the parameter array |
| 179 | int job = parallelJobs.numberOfJobs(); |
| 180 | if (job > 1) { |
| 181 | int yStart = 1; |
| 182 | - int yStep = (data.heightDecreasedByOne - 1) / job; |
| 183 | + int yStep = (data.height - 2) / job; |
| 184 | for (--job; job >= 0; --job) { |
| 185 | FELightingPaintingDataForNeon& params = parallelJobs.parameter(job); |
| 186 | params = neonData; |
| 187 | params.yStart = yStart; |
| 188 | - params.pixels += (yStart - 1) * (data.widthDecreasedByOne + 1) * 4; |
| 189 | + params.pixels += (yStart - 1) * data.width * 4; |
| 190 | if (job > 0) { |
| 191 | params.absoluteHeight = yStep; |
| 192 | yStart += yStep; |
| 193 | } else |
| 194 | - params.absoluteHeight = data.heightDecreasedByOne - yStart; |
| 195 | + params.absoluteHeight = (data.height - 1) - yStart; |
| 196 | } |
| 197 | parallelJobs.execute(); |
| 198 | return; |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 199 | @@ -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] | 200 | } // namespace WebCore |
| 201 | |
| 202 | #endif // CPU(ARM_NEON) && COMPILER(GCC_COMPATIBLE) |
| 203 | - |
| 204 | -#endif // FELightingNEON_h |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 205 | diff --git a/Source/WebCore/platform/graphics/filters/DistantLightSource.h b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| 206 | index 70c6512f..b032c82e 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 207 | --- a/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| 208 | +++ b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 209 | @@ -26,6 +26,10 @@ |
| 210 | #include <wtf/ArgumentCoder.h> |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 211 | #include <wtf/Ref.h> |
| 212 | |
| 213 | +namespace WTF { |
| 214 | +class TextStream; |
| 215 | +} // namespace WTF |
| 216 | + |
| 217 | namespace WebCore { |
| 218 | |
| 219 | class DistantLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 220 | diff --git a/Source/WebCore/platform/graphics/filters/FELighting.h b/Source/WebCore/platform/graphics/filters/FELighting.h |
| 221 | index 53beb596..e78a9354 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 222 | --- a/Source/WebCore/platform/graphics/filters/FELighting.h |
| 223 | +++ b/Source/WebCore/platform/graphics/filters/FELighting.h |
| 224 | @@ -35,8 +35,6 @@ |
| 225 | |
| 226 | namespace WebCore { |
| 227 | |
| 228 | -struct FELightingPaintingDataForNeon; |
| 229 | - |
| 230 | class FELighting : public FilterEffect { |
| 231 | public: |
| 232 | const Color& lightingColor() const { return m_lightingColor; } |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 233 | @@ -64,11 +62,6 @@ protected: |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 234 | |
| 235 | std::unique_ptr<FilterEffectApplier> createSoftwareApplier() const override; |
| 236 | |
| 237 | -#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 238 | - static int getPowerCoefficients(float exponent); |
| 239 | - inline void platformApplyNeon(const LightingData&, const LightSource::PaintingData&); |
| 240 | -#endif |
| 241 | - |
| 242 | Color m_lightingColor; |
| 243 | float m_surfaceScale; |
| 244 | float m_diffuseConstant; |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 245 | diff --git a/Source/WebCore/platform/graphics/filters/PointLightSource.h b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| 246 | index 3a5723f0..675d63f5 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 247 | --- a/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| 248 | +++ b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| 249 | @@ -26,6 +26,10 @@ |
| 250 | #include "LightSource.h" |
| 251 | #include <wtf/Ref.h> |
| 252 | |
| 253 | +namespace WTF { |
| 254 | +class TextStream; |
| 255 | +} // namespace WTF |
| 256 | + |
| 257 | namespace WebCore { |
| 258 | |
| 259 | class PointLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 260 | diff --git a/Source/WebCore/platform/graphics/filters/SpotLightSource.h b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| 261 | index 684626f7..dea58389 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 262 | --- a/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| 263 | +++ b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| 264 | @@ -26,6 +26,10 @@ |
| 265 | #include "LightSource.h" |
| 266 | #include <wtf/Ref.h> |
| 267 | |
| 268 | +namespace WTF { |
| 269 | +class TextStream; |
| 270 | +} // namespace WTF |
| 271 | + |
| 272 | namespace WebCore { |
| 273 | |
| 274 | class SpotLightSource : public LightSource { |
Andrew Geissler | 8f84068 | 2023-07-21 09:09:43 -0500 | [diff] [blame] | 275 | diff --git a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 276 | index c974d921..e2896660 100644 |
Andrew Geissler | 87f5cff | 2022-09-30 13:13:31 -0500 | [diff] [blame] | 277 | --- a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 278 | +++ b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| 279 | @@ -36,6 +36,7 @@ |
| 280 | namespace WebCore { |
| 281 | |
| 282 | class FELighting; |
| 283 | +struct FELightingPaintingDataForNeon; |
| 284 | |
| 285 | class FELightingSoftwareApplier final : public FilterEffectConcreteApplier<FELighting> { |
| 286 | WTF_MAKE_FAST_ALLOCATED; |
| 287 | @@ -132,8 +133,23 @@ private: |
| 288 | |
| 289 | static void applyPlatformGenericPaint(const LightingData&, const LightSource::PaintingData&, int startY, int endY); |
| 290 | static void applyPlatformGenericWorker(ApplyParameters*); |
| 291 | + |
| 292 | +#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| 293 | + static int getPowerCoefficients(float exponent); |
| 294 | + static void platformApplyNeonWorker(FELightingPaintingDataForNeon*); |
| 295 | + inline static void applyPlatformNeon(const LightingData&, const LightSource::PaintingData&); |
| 296 | + |
| 297 | + inline static void applyPlatformGeneric(const LightingData& data, const LightSource::PaintingData& paintingData) |
| 298 | + { |
| 299 | + applyPlatformNeon(data, paintingData); |
| 300 | + } |
| 301 | +#else |
| 302 | static void applyPlatformGeneric(const LightingData&, const LightSource::PaintingData&); |
| 303 | +#endif |
| 304 | + |
| 305 | static void applyPlatform(const LightingData&); |
| 306 | }; |
| 307 | |
| 308 | } // namespace WebCore |
| 309 | + |
| 310 | +#include "FELightingNEON.h" |