Closed
Bug 563488
Opened 14 years ago
Closed 14 years ago
Make pixman support self-copy operation
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 635035
People
(Reporter: mattwoodrow, Unassigned)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Pixman should be able to support self-copy operations and preferably without needing to make a temporary copy of the surface.
Reporter | ||
Comment 1•14 years ago
|
||
This appears to make self copy work flawlessly in the simple cases.
I still need to confirm that the functions with implemented self copy support (general and sse2 rgba->rgba) can handle tiny shifts.
Obviously needs a style cleanup before it's complete.
Attachment #443203 -
Flags: review?(roc)
+ int sum_x = dest_x - src_x;
+ int sum_y = dest_y - src_y;
These are more often called "delta"
+ /* Same dest as source, positive scroll */
+ pixman_bool_t forwardY = !((dest == src) && (sum_y > 0));
+ pixman_bool_t forwardX = !((dest == src) && (sum_x > 0));
canScanForwardX/Y?
Something needs to happen here to handle generic cases like when we rotate a buffer by 90 degrees in-place. We definitely need to be creating a temporary surface sometimes.
Reporter | ||
Comment 3•14 years ago
|
||
Added creation of a temporary surface when a translation is required.
Added X translation support to the SSE2 backend function.
Tidied naming/comments.
Tested with various self copies including forward X/Y (both large numbers and 1px), and rotation.
Attachment #443203 -
Attachment is obsolete: true
Attachment #443203 -
Flags: review?(roc)
Comment 4•14 years ago
|
||
Comment 5•14 years ago
|
||
Comment on attachment 443561 [details] [diff] [review]
same patch but with -p -U8 ctx
oh, wrong patch
Attachment #443561 -
Attachment is obsolete: true
Comment 6•14 years ago
|
||
Comment 7•14 years ago
|
||
I think it is a bit problematic to apply this patch on current central repo
bug 562087 need to be landed first
Depends on: 562087
Comment 8•14 years ago
|
||
Also, this should go upstream before we land it in central. I don't want to have to maintain it separately.
Suggest blocking fennec-final, because this is an optimization transparent to
gecko code that should be a big win for scrolling perf, when that's tuned.
tracking-fennec: --- → ?
Updated•14 years ago
|
tracking-fennec: ? → 2.0+
Comment 11•14 years ago
|
||
(In reply to comment #3)
> Created attachment 443249 [details] [diff] [review]
> Fixed handling of translations
>
> Added creation of a temporary surface when a translation is required.
> Added X translation support to the SSE2 backend function.
> Tidied naming/comments.
>
> Tested with various self copies including forward X/Y (both large numbers and
> 1px), and rotation.
As was discussed on #cairo irc channel a long time ago, everyone agreed (most importantly Søren Sandmann) that there are no strong objections and self-copy support can be added to pixman. But the patch is just a bit intrusive in the sense that it touches almost *every* possible way of using of pixman and needs to be tested really well. And an automated test for pixman test suite which would stress self-copy is a must.
The statement "It doesn't have to be complex" [1] is not very convincing at least to me without having a test program which can stress many possible self-copy related corner cases of pixman usage to support it. In particular, I feel that having complex clipping may (or may not) expose some bugs. Being pedantic, lots of other ways of using pixman (setting custom accessors, etc.) should be tested too.
The other relatively minor thing (but important for getting the patch accepted) is that some coding style [2] issues need to be fixed.
1. http://lists.cairographics.org/archives/cairo/2010-May/019816.html
2. http://cgit.freedesktop.org/pixman/tree/CODING_STYLE
Comment 12•14 years ago
|
||
oleg, do you have tests for this?
Updated•14 years ago
|
tracking-fennec: 2.0+ → 2.0-
Comment 13•14 years ago
|
||
(In reply to comment #10)
> Suggest blocking fennec-final, because this is an optimization transparent to
> gecko code that should be a big win for scrolling perf, when that's tuned.
Is still needed?
tracking-fennec: 2.0- → ---
Updated•14 years ago
|
Whiteboard: [fennec-4.1?]
It's not important for scrolling perf in Gecko anymore; we wrote a very simple self-copy impl for the simple case we care most about (bug 635035).
Updated•14 years ago
|
Whiteboard: [fennec-4.1?]
Reporter | ||
Comment 15•14 years ago
|
||
Fixed by bug 635035.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Resolution: FIXED → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•