Closed Bug 640250 Opened 14 years ago Closed 14 years ago

Update pixman for Firefox 5

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch Update pixman to 17feaa9... (obsolete) (deleted) — Splinter Review
This includes more sse2 and neon optimizations and some gradient changes.
I've an update to this that fixes a build failure and a reftest failure
This crashes on win32. 0 xul.dll!in_over_1x128 [pixman-sse2.c:b8e874ec45e1 : 436 + 0x11] eip = 0x69a07097 esp = 0x001ea0f8 ebp = 0x001ea118 ebx = 0x001ea280 esi = 0x001eb190 edi = 0x001eb1c8 eax = 0x001ea1b0 ecx = 0x001ea1c0 edx = 0x001ea1f0 efl = 0x00010202
This seems like a compiler bug. MSVC 2008 and presumably 2005 seem to generate a movdqa to a unaligned address. _in_over_1x128: 55 push ebp // esp becomes unaligned 8B EC mov ebp,esp // ebp becomes unaligned 83 EC 20 sub esp,20h 8B 45 14 mov eax,dword ptr [ebp+14h] 66 0F 6F 00 movdqa xmm0,xmmword ptr [eax] 8B 4D 10 mov ecx,dword ptr [ebp+10h] 66 0F 6F 09 movdqa xmm1,xmmword ptr [ecx] 8B 55 0C mov edx,dword ptr [ebp+0Ch] 66 0F 7F 45 F0 movdqa xmmword ptr [ebp-10h],xmm0 // we fault here because ebp is unaligned [snip] MSVC 2010 generates code that seems to align the stack properly. _in_over_1x128: 00000000: 53 push ebx 00000001: 8B DC mov ebx,esp 00000003: 83 EC 08 sub esp,8 00000006: 83 E4 F0 and esp,0FFFFFFF0h 00000009: 83 C4 04 add esp,4 0000000C: 55 push ebp 0000000D: 8B 6B 04 mov ebp,dword ptr [ebx+4] 00000010: 89 6C 24 04 mov dword ptr [esp+4],ebp 00000014: 8B EC mov ebp,esp 00000016: 83 EC 20 sub esp,20h 00000019: 8B 43 14 mov eax,dword ptr [ebx+14h] 0000001C: 66 0F 6F 00 movdqa xmm0,xmmword ptr [eax] 00000020: 8B 4B 10 mov ecx,dword ptr [ebx+10h] 00000023: 66 0F 6F 09 movdqa xmm1,xmmword ptr [ecx] 00000027: 8B 53 0C mov edx,dword ptr [ebx+0Ch] 0000002A: 66 0F 7F 45 F0 movdqa xmmword ptr [ebp-10h],xmm0 0000002F: 66 0F 6F 02 movdqa xmm0,xmmword ptr [edx] 00000033: E8 00 00 00 00 call _pix_multiply_1x128 00000038: 8B 43 10 mov eax,dword ptr [ebx+10h] 0000003B: 66 0F 6F 08 movdqa xmm1,xmmword ptr [eax] 0000003F: 8B 4B 08 mov ecx,dword ptr [ebx+8] 00000042: 66 0F 7F 45 E0 movdqa xmmword ptr [ebp-20h],xmm0 00000047: 66 0F 6F 01 movdqa xmm0,xmmword ptr [ecx] 0000004B: E8 00 00 00 00 call _pix_multiply_1x128 00000050: 66 0F 6F 4D E0 movdqa xmm1,xmmword ptr [ebp-20h] 00000055: 66 0F 6F 55 F0 movdqa xmm2,xmmword ptr [ebp-10h] 0000005A: E8 00 00 00 00 call _over_1x128 0000005F: 8B E5 mov esp,ebp 00000061: 5D pop ebp 00000062: 8B E3 mov esp,ebx 00000064: 5B pop ebx 00000065: C3 ret
Attached patch v2 (obsolete) (deleted) — Splinter Review
This version builds everywhere. The first talos numbers suggest that this is causing a noticeable regression of tp4 http://tinyurl.com/6xfoxal. I'll have to investigate further.
Attachment #518122 - Attachment is obsolete: true
That's interesting, I wonder if the performance regression could be related to http://cgit.freedesktop.org/pixman/commit/?id=73c1fefa1b99efa36b74599f455df9426209378e I also have a test branch with this patch reverted (additionally setting FAST_PATH_SAMPLES_COVER_CLIP flag is also needed): http://cgit.freedesktop.org/~siamashka/pixman/commit/?h=playground/revert-simple-repeat-removal&id=3f3c58470cac16c4af91264d7b18869679e9f89e
And additionally radial gradients are now supposed to be more correct, but slower: http://lists.freedesktop.org/archives/pixman/2011-January/000838.html
(In reply to comment #6) > That's interesting, I wonder if the performance regression could be related to > http://cgit.freedesktop.org/pixman/commit/?id=73c1fefa1b99efa36b74599f455df9426209378e > > I also have a test branch with this patch reverted (additionally setting > FAST_PATH_SAMPLES_COVER_CLIP flag is also needed): > http://cgit.freedesktop.org/~siamashka/pixman/commit/?h=playground/revert-simple-repeat-removal&id=3f3c58470cac16c4af91264d7b18869679e9f89e I'll give this version a try.
Blocks: 635463
Here are the new results: http://tinyurl.com/6gn464h It seems like reverting that change does fix the regression.
Attached patch v3 (deleted) — Splinter Review
Attachment #518778 - Attachment is obsolete: true
Assignee: nobody → jmuizelaar
This patch is also important in order to get faster video for Fennec (software rendering)
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
This caused a noticeable regression with svg on the n900 see bug 646299
Blocks: 628324
Depends on: 656782
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: