Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[wpe-2.38] Regression in SVG lightning filter #1145

Closed
Scony opened this issue Aug 24, 2023 · 14 comments · Fixed by #1199
Closed

[wpe-2.38] Regression in SVG lightning filter #1145

Scony opened this issue Aug 24, 2023 · 14 comments · Fixed by #1199
Assignees
@magomez
Labels
wpe-2.38 wpe-2.42

Comments

@Scony
Copy link

Scony commented Aug 24, 2023

To reproduce, run ACT-1345 test case from BBC cert suite, for that run:

2.28

228

2.38

238

Please note that the issue happens regardless of LBSE being used or not (ENABLE_LAYER_BASED_SVG_ENGINE option).
@github-actions
Copy link

Please add a wpe-x.xx version label to the issue.

@Scony
Copy link
Author

Scony commented Aug 24, 2023

Please add a wpe-x.xx version label to the issue.

I can't add labels...

@woutermeek
Copy link

Oh, well that is not good :) Let me check that.

@Scony
Copy link
Author

Scony commented Sep 21, 2023

Direct link to problematic SVG: https://certification.bbctvapps.co.uk/act/test/ui/187/svg-supports-filter-lighting/actual-lighting.svg

Scony added a commit to Scony/WPEWebKit that referenced this issue Sep 27, 2023
@Scony
Add missing feComposite filter's arithmetic operation implementation, f…
85e9f2e 
…ixes WebPlatformForEmbedded#1145

On ARM platforms having neon intrinsics ABI available the implementation of
platfrom-specific arithmetic operation of feComposite filter was not hooked
despite the fact that implementation was in place - it was probably missed
during cherry-picking.
@Scony Scony mentioned this issue Sep 27, 2023
Merged
magomez pushed a commit that referenced this issue Sep 29, 2023
@Scony @magomez
Add missing feComposite filter's arithmetic operation implementation, f…
4781435 
…ixes #1145

On ARM platforms having neon intrinsics ABI available the implementation of
platfrom-specific arithmetic operation of feComposite filter was not hooked
despite the fact that implementation was in place - it was probably missed
during cherry-picking.
@pgorszkowski-igalia pgorszkowski-igalia mentioned this issue Nov 14, 2023
Merged
pgorszkowski-igalia added a commit that referenced this issue Nov 15, 2023
@pgorszkowski-igalia
Revert "Add missing feComposite filter's arithmetic operation impleme…
a7b82b0 
…ntation, fixes #1145"

This reverts commit 85e9f2e.
pgorszkowski-igalia added a commit that referenced this issue Nov 15, 2023
@pgorszkowski-igalia
Revert "Add missing feComposite filter's arithmetic operation impleme…
e39032e 
…ntation, fixes #1145"

This reverts commit 4781435.
@pgorszkowski-igalia
Copy link

I backported the fix from the upstream which implements the same thing in clearer and proper way:
wpe-2.38: #1235
wpe-2.42: #1236

@Scony
Copy link
Author

Scony commented Nov 20, 2023

Thanks! I'll give it a try in upcoming week or two.

@amol-virnodkar-infosys
Copy link

amol-virnodkar-infosys commented Dec 20, 2023 •
edited
Loading

I ported the fix(#1235) with WPE 2.38 and tested the fix on a Broadcom based device, we are observing the issue in WPE 2.38 as below. The shades are bit darker in colour.

svg

We will be verifying this fix on a reference video accelerator device soon.
@pgorszkowski-igalia Could you please check the behavior in the above image and provide your inputs.

@Scony
Copy link
Author

Scony commented Dec 20, 2023

@amol-virnodkar-infosys in the Firefox the shades are bit darker as well.

@Scony
Copy link
Author

Scony commented Dec 20, 2023

@pgorszkowski-igalia I forgot to mention - few weeks ago I was testing your changes and it's all good.

@rychem
Copy link

rychem commented Jan 8, 2024

@pgorszkowski-igalia in context of darker 'actual rendering' reported by @amol-virnodkar-infosys - test results for wpe2.22 are the same as indicated above for 2.28 (expected=actual)

@pgorszkowski-igalia
Copy link

@amol-virnodkar-infosys , @rychem : I can confirm that in case of wpe 2.38 the actual results are a bit darker (RPi3B+).

It seems that also in upstream FELighting does not work correctly for ARM NEON. It is another part which does not work after refactoring change (similar to the FEComposite things). The fix will be provided in upstream and then backported.

@amol-virnodkar-infosys: nice finding, thanks

@rychem
Copy link

rychem commented Jan 11, 2024

Thanks @pgorszkowski-igalia for confirmation.
Is this ticket supposed to be reopened or other ticket is created for trackability ... ?

@pgorszkowski-igalia
Copy link

@rychem : maybe it would be better to create a separate ticket for it. Thanks.

@amol-virnodkar-infosys
Copy link

@pgorszkowski-igalia Created a new ticket - #1274 as suggested by you. Thanks.

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

@magomez magomez

Labels
wpe-2.38 wpe-2.42
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[2.38] Add missing feComposite filter's arithmetic operation implementation Scony/WPEWebKit
6 participants
@woutermeek @Scony @magomez @rychem @pgorszkowski-igalia @amol-virnodkar-infosys