Closed Bug 869178 Opened 6 years ago Closed 6 years ago

[Skia] Use drawBitmapRect instead of creating a bitmap shader when drawing images in DrawTargetSkia

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: gw280, Assigned: gw280)

References

Details

Attachments

(1 file)

drawBitmap* methods on SkCanvas support slicing up bitmaps larger than the maximum texture size when using the GL backend for Skia, but using bitmap shaders does not. We should therefore use drawBitmapRect.
Attachment #746059 - Flags: review?(snorp)
Comment on attachment 746059 [details] [diff] [review]
Use drawBitmapRect

Review of attachment 746059 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Does this fix some of the drawImage() failures I've seen on some things?
Attachment #746059 - Flags: review?(snorp) → review+
It should fix any instance of the "failed to create texture for bitmap" type error.
https://hg.mozilla.org/mozilla-central/rev/f9a17848125a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Backed out for reftest crashes. Thanks for watching your push after landing on m-c, btw.
https://hg.mozilla.org/mozilla-central/rev/b25afb305360

https://tbpl.mozilla.org/php/getParsedLog.php?id=22711192&tree=Mozilla-Central
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> Backed out for reftest crashes. Thanks for watching your push after landing
> on m-c, btw.

I was watching, just got back a little bit too late. Very weird, because the patches passed try without issue... Hrmmmmm...
    // The reason we do clipping is because Skia doesn't support SkRect as                                                                                                        
    // source rect. See http://crbug.com/145540.                                                                                                                                  
    // When Skia supports then use this as the source rect to replace 0.                                                                                                          
    //                                                                                                                                                                            
    // scaledSrcRect.offset(-enclosingScaledSrcRect.x(), -enclosingScaledSrcRect.y());                                                                                            
    context->save();
    context->clipRect(destRectVisibleSubset);

    // Because the image fragment is generated with an approxmiated scaling                                                                                                       
    // factor. This draw will perform a close to 1 scaling.                                                                                                                       
    //                                                                                                                                                                            
    // NOTE: For future optimization. If the difference in scale is so small                                                                                                      
    // that Skia doesn't produce a difference then we can just blit it directly                                                                                                   
    // to enhance performance.                                                                                                                                                    
    context->drawBitmapRect(scaledImageFragment, 0, enclosingDestRect, &paint);
    context->restore();

I think you are failing the tests because you just truncate the source rect. We should do what chrome does here.
The current version of the patch that is on the graphics branch does what Chrome does.
https://hg.mozilla.org/mozilla-central/rev/f5cc97a9ea30
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.