Closed Bug 640250 Opened 13 years ago Closed 13 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) — 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) — 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 v3Splinter 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)
http://hg.mozilla.org/mozilla-central/rev/820512b8223e
Status: NEW → RESOLVED
Closed: 13 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.