| From 647c93de99a0f71f478d76a4cc7714eba7ba1447 Mon Sep 17 00:00:00 2001 |
| From: Adrian Perez de Castro <aperez@igalia.com> |
| Date: Thu, 2 Jun 2022 11:19:06 +0300 |
| Subject: [PATCH] FELightningNEON.cpp fails to build, NEON fast path seems |
| unused https://bugs.webkit.org/show_bug.cgi?id=241182 |
| |
| Reviewed by NOBODY (OOPS!). |
| |
| Move the NEON fast path for the SVG lighting filter effects into |
| FELightingSoftwareApplier, and arrange to actually use them by |
| forwarding calls to applyPlatformGeneric() into applyPlatformNeon(). |
| |
| Some changes were needed to adapt platformApplyNeon() to the current |
| state of filters after r286140. This was not detected because the code |
| bitrotted due to it being guarded with CPU(ARM_TRADITIONAL), which does |
| not get used much these days: CPU(ARM_THUMB2) is more common. It should |
| be possible to use the NEON fast paths also in Thumb mode, but that is |
| left for a follow-up fix. |
| |
| * Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp: |
| (WebCore::FELightingSoftwareApplier::platformApplyNeonWorker): |
| (WebCore::FELightingSoftwareApplier::getPowerCoefficients): |
| (WebCore::FELighting::platformApplyNeonWorker): Deleted. |
| (WebCore::FELighting::getPowerCoefficients): Deleted. |
| * Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h: |
| (WebCore::FELightingSoftwareApplier::applyPlatformNeon): |
| (WebCore::FELighting::platformApplyNeon): Deleted. |
| * Source/WebCore/platform/graphics/filters/DistantLightSource.h: |
| * Source/WebCore/platform/graphics/filters/FELighting.h: |
| * Source/WebCore/platform/graphics/filters/PointLightSource.h: |
| * Source/WebCore/platform/graphics/filters/SpotLightSource.h: |
| * Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h: |
| |
| Upstream-Status: Submitted [https://github.com/WebKit/WebKit/pull/1233] |
| Signed-off-by: Khem Raj <raj.khem@gmail.com> |
| --- |
| .../cpu/arm/filters/FELightingNEON.cpp | 4 +- |
| .../graphics/cpu/arm/filters/FELightingNEON.h | 52 +++++++++---------- |
| .../graphics/filters/DistantLightSource.h | 4 ++ |
| .../platform/graphics/filters/FELighting.h | 7 --- |
| .../graphics/filters/PointLightSource.h | 4 ++ |
| .../graphics/filters/SpotLightSource.h | 4 ++ |
| .../software/FELightingSoftwareApplier.h | 16 ++++++ |
| 7 files changed, 56 insertions(+), 35 deletions(-) |
| |
| diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| index f6ff8c20..42a97ffc 100644 |
| --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp |
| @@ -49,7 +49,7 @@ short* feLightingConstantsForNeon() |
| return s_FELightingConstantsForNeon; |
| } |
| |
| -void FELighting::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| +void FELightingSoftwareApplier::platformApplyNeonWorker(FELightingPaintingDataForNeon* parameters) |
| { |
| neonDrawLighting(parameters); |
| } |
| @@ -464,7 +464,7 @@ TOSTRING(neonDrawLighting) ":" NL |
| "b .lightStrengthCalculated" NL |
| ); // NOLINT |
| |
| -int FELighting::getPowerCoefficients(float exponent) |
| +int FELightingSoftwareApplier::getPowerCoefficients(float exponent) |
| { |
| // Calling a powf function from the assembly code would require to save |
| // and reload a lot of NEON registers. Since the base is in range [0..1] |
| diff --git a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| index b17c603d..e4629cda 100644 |
| --- a/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| +++ b/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h |
| @@ -24,14 +24,15 @@ |
| * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. |
| */ |
| |
| -#ifndef FELightingNEON_h |
| -#define FELightingNEON_h |
| +#pragma once |
| |
| #if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| |
| -#include "FELighting.h" |
| +#include "FELightingSoftwareApplier.h" |
| +#include "ImageBuffer.h" |
| #include "PointLightSource.h" |
| #include "SpotLightSource.h" |
| +#include <wtf/ObjectIdentifier.h> |
| #include <wtf/ParallelJobs.h> |
| |
| namespace WebCore { |
| @@ -93,14 +94,14 @@ extern "C" { |
| void neonDrawLighting(FELightingPaintingDataForNeon*); |
| } |
| |
| -inline void FELighting::platformApplyNeon(const LightingData& data, const LightSource::PaintingData& paintingData) |
| +inline void FELightingSoftwareApplier::applyPlatformNeon(const FELightingSoftwareApplier::LightingData& data, const LightSource::PaintingData& paintingData) |
| { |
| - alignas(16) FELightingFloatArgumentsForNeon floatArguments; |
| - FELightingPaintingDataForNeon neonData = { |
| + WebCore::FELightingFloatArgumentsForNeon alignas(16) floatArguments; |
| + WebCore::FELightingPaintingDataForNeon neonData = { |
| data.pixels->data(), |
| 1, |
| - data.widthDecreasedByOne - 1, |
| - data.heightDecreasedByOne - 1, |
| + data.width - 2, |
| + data.height - 2, |
| 0, |
| 0, |
| 0, |
| @@ -111,23 +112,23 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
| // Set light source arguments. |
| floatArguments.constOne = 1; |
| |
| - auto color = m_lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| + auto color = data.lightingColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved(); |
| |
| floatArguments.colorRed = color.red; |
| floatArguments.colorGreen = color.green; |
| floatArguments.colorBlue = color.blue; |
| floatArguments.padding4 = 0; |
| |
| - if (m_lightSource->type() == LS_POINT) { |
| + if (data.lightSource->type() == LS_POINT) { |
| neonData.flags |= FLAG_POINT_LIGHT; |
| - PointLightSource& pointLightSource = static_cast<PointLightSource&>(m_lightSource.get()); |
| + const auto& pointLightSource = *static_cast<const PointLightSource*>(data.lightSource); |
| floatArguments.lightX = pointLightSource.position().x(); |
| floatArguments.lightY = pointLightSource.position().y(); |
| floatArguments.lightZ = pointLightSource.position().z(); |
| floatArguments.padding2 = 0; |
| - } else if (m_lightSource->type() == LS_SPOT) { |
| + } else if (data.lightSource->type() == LS_SPOT) { |
| neonData.flags |= FLAG_SPOT_LIGHT; |
| - SpotLightSource& spotLightSource = static_cast<SpotLightSource&>(m_lightSource.get()); |
| + const auto& spotLightSource = *static_cast<const SpotLightSource*>(data.lightSource); |
| floatArguments.lightX = spotLightSource.position().x(); |
| floatArguments.lightY = spotLightSource.position().y(); |
| floatArguments.lightZ = spotLightSource.position().z(); |
| @@ -145,7 +146,7 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
| if (spotLightSource.specularExponent() == 1) |
| neonData.flags |= FLAG_CONE_EXPONENT_IS_1; |
| } else { |
| - ASSERT(m_lightSource->type() == LS_DISTANT); |
| + ASSERT(data.lightSource->type() == LS_DISTANT); |
| floatArguments.lightX = paintingData.initialLightingData.lightVector.x(); |
| floatArguments.lightY = paintingData.initialLightingData.lightVector.y(); |
| floatArguments.lightZ = paintingData.initialLightingData.lightVector.z(); |
| @@ -155,38 +156,39 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
| // Set lighting arguments. |
| floatArguments.surfaceScale = data.surfaceScale; |
| floatArguments.minusSurfaceScaleDividedByFour = -data.surfaceScale / 4; |
| - if (m_lightingType == FELighting::DiffuseLighting) |
| - floatArguments.diffuseConstant = m_diffuseConstant; |
| + if (data.filterType == FilterEffect::Type::FEDiffuseLighting) |
| + floatArguments.diffuseConstant = data.diffuseConstant; |
| else { |
| neonData.flags |= FLAG_SPECULAR_LIGHT; |
| - floatArguments.diffuseConstant = m_specularConstant; |
| - neonData.specularExponent = getPowerCoefficients(m_specularExponent); |
| - if (m_specularExponent == 1) |
| + floatArguments.diffuseConstant = data.specularConstant; |
| + neonData.specularExponent = getPowerCoefficients(data.specularExponent); |
| + if (data.specularExponent == 1) |
| neonData.flags |= FLAG_SPECULAR_EXPONENT_IS_1; |
| } |
| if (floatArguments.diffuseConstant == 1) |
| neonData.flags |= FLAG_DIFFUSE_CONST_IS_1; |
| |
| - int optimalThreadNumber = ((data.widthDecreasedByOne - 1) * (data.heightDecreasedByOne - 1)) / s_minimalRectDimension; |
| + static constexpr int minimalRectDimension = 100 * 100; // Empirical data limit for parallel jobs |
| + int optimalThreadNumber = ((data.width - 2) * (data.height - 2)) / minimalRectDimension; |
| if (optimalThreadNumber > 1) { |
| // Initialize parallel jobs |
| - ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&WebCore::FELighting::platformApplyNeonWorker, optimalThreadNumber); |
| + ParallelJobs<FELightingPaintingDataForNeon> parallelJobs(&FELightingSoftwareApplier::platformApplyNeonWorker, optimalThreadNumber); |
| |
| // Fill the parameter array |
| int job = parallelJobs.numberOfJobs(); |
| if (job > 1) { |
| int yStart = 1; |
| - int yStep = (data.heightDecreasedByOne - 1) / job; |
| + int yStep = (data.height - 2) / job; |
| for (--job; job >= 0; --job) { |
| FELightingPaintingDataForNeon& params = parallelJobs.parameter(job); |
| params = neonData; |
| params.yStart = yStart; |
| - params.pixels += (yStart - 1) * (data.widthDecreasedByOne + 1) * 4; |
| + params.pixels += (yStart - 1) * data.width * 4; |
| if (job > 0) { |
| params.absoluteHeight = yStep; |
| yStart += yStep; |
| } else |
| - params.absoluteHeight = data.heightDecreasedByOne - yStart; |
| + params.absoluteHeight = (data.height - 1) - yStart; |
| } |
| parallelJobs.execute(); |
| return; |
| @@ -199,5 +201,3 @@ inline void FELighting::platformApplyNeon(const LightingData& data, const LightS |
| } // namespace WebCore |
| |
| #endif // CPU(ARM_NEON) && COMPILER(GCC_COMPATIBLE) |
| - |
| -#endif // FELightingNEON_h |
| diff --git a/Source/WebCore/platform/graphics/filters/DistantLightSource.h b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| index 70c6512f..b032c82e 100644 |
| --- a/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| +++ b/Source/WebCore/platform/graphics/filters/DistantLightSource.h |
| @@ -26,6 +26,10 @@ |
| #include <wtf/ArgumentCoder.h> |
| #include <wtf/Ref.h> |
| |
| +namespace WTF { |
| +class TextStream; |
| +} // namespace WTF |
| + |
| namespace WebCore { |
| |
| class DistantLightSource : public LightSource { |
| diff --git a/Source/WebCore/platform/graphics/filters/FELighting.h b/Source/WebCore/platform/graphics/filters/FELighting.h |
| index 53beb596..e78a9354 100644 |
| --- a/Source/WebCore/platform/graphics/filters/FELighting.h |
| +++ b/Source/WebCore/platform/graphics/filters/FELighting.h |
| @@ -35,8 +35,6 @@ |
| |
| namespace WebCore { |
| |
| -struct FELightingPaintingDataForNeon; |
| - |
| class FELighting : public FilterEffect { |
| public: |
| const Color& lightingColor() const { return m_lightingColor; } |
| @@ -64,11 +62,6 @@ protected: |
| |
| std::unique_ptr<FilterEffectApplier> createSoftwareApplier() const override; |
| |
| -#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| - static int getPowerCoefficients(float exponent); |
| - inline void platformApplyNeon(const LightingData&, const LightSource::PaintingData&); |
| -#endif |
| - |
| Color m_lightingColor; |
| float m_surfaceScale; |
| float m_diffuseConstant; |
| diff --git a/Source/WebCore/platform/graphics/filters/PointLightSource.h b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| index 3a5723f0..675d63f5 100644 |
| --- a/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| +++ b/Source/WebCore/platform/graphics/filters/PointLightSource.h |
| @@ -26,6 +26,10 @@ |
| #include "LightSource.h" |
| #include <wtf/Ref.h> |
| |
| +namespace WTF { |
| +class TextStream; |
| +} // namespace WTF |
| + |
| namespace WebCore { |
| |
| class PointLightSource : public LightSource { |
| diff --git a/Source/WebCore/platform/graphics/filters/SpotLightSource.h b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| index 684626f7..dea58389 100644 |
| --- a/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| +++ b/Source/WebCore/platform/graphics/filters/SpotLightSource.h |
| @@ -26,6 +26,10 @@ |
| #include "LightSource.h" |
| #include <wtf/Ref.h> |
| |
| +namespace WTF { |
| +class TextStream; |
| +} // namespace WTF |
| + |
| namespace WebCore { |
| |
| class SpotLightSource : public LightSource { |
| diff --git a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| index c974d921..e2896660 100644 |
| --- a/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| +++ b/Source/WebCore/platform/graphics/filters/software/FELightingSoftwareApplier.h |
| @@ -36,6 +36,7 @@ |
| namespace WebCore { |
| |
| class FELighting; |
| +struct FELightingPaintingDataForNeon; |
| |
| class FELightingSoftwareApplier final : public FilterEffectConcreteApplier<FELighting> { |
| WTF_MAKE_FAST_ALLOCATED; |
| @@ -132,8 +133,23 @@ private: |
| |
| static void applyPlatformGenericPaint(const LightingData&, const LightSource::PaintingData&, int startY, int endY); |
| static void applyPlatformGenericWorker(ApplyParameters*); |
| + |
| +#if CPU(ARM_NEON) && CPU(ARM_TRADITIONAL) && COMPILER(GCC_COMPATIBLE) |
| + static int getPowerCoefficients(float exponent); |
| + static void platformApplyNeonWorker(FELightingPaintingDataForNeon*); |
| + inline static void applyPlatformNeon(const LightingData&, const LightSource::PaintingData&); |
| + |
| + inline static void applyPlatformGeneric(const LightingData& data, const LightSource::PaintingData& paintingData) |
| + { |
| + applyPlatformNeon(data, paintingData); |
| + } |
| +#else |
| static void applyPlatformGeneric(const LightingData&, const LightSource::PaintingData&); |
| +#endif |
| + |
| static void applyPlatform(const LightingData&); |
| }; |
| |
| } // namespace WebCore |
| + |
| +#include "FELightingNEON.h" |