Skip to content

Commit

Permalink
Removing workaround in PaintCanvasVideoRenderer::Paint
Browse files Browse the repository at this point in the history
We are now using SharedImage to hold the image for the video renderer,
https://chromium-review.googlesource.com/c/chromium/src/+/1616978.
That change, together with the change that made the video be deferred, and
therefore, use the paintRecorder, let us safely remove the workaround.

We are adding some comments to explain that removal, in case that this is
stopped being deferred at any point.

Bug: 974656
Change-Id: If050f1f12b098709e20547cb56709d4e5381af90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733438
Commit-Queue: Juanmi Huertas <juanmihd@chromium.org>
Commit-Queue: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683707}
  • Loading branch information
Juanmihd authored and Commit Bot committed Aug 2, 2019
1 parent 812103a commit 0cfb2d5
Showing 1 changed file with 8 additions and 14 deletions.
22 changes: 8 additions & 14 deletions media/renderers/paint_canvas_video_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -634,20 +634,14 @@ void PaintCanvasVideoRenderer::Paint(scoped_refptr<VideoFrame> video_frame,
-SkFloatToScalar(image.height() * 0.5f));
}

// This is a workaround for crbug.com/524717. A texture backed image is not
// safe to access on another thread or GL context. So if we're drawing into a
// recording canvas we read the texture back into CPU memory and record that
// sw image into the SkPicture. The long term solution is for Skia to provide
// a SkPicture filter that makes a picture safe for multiple CPU raster
// threads. (skbug.com/4321).
if (canvas->imageInfo().colorType() == kUnknown_SkColorType &&
image.IsTextureBacked()) {
sk_sp<SkImage> non_texture_image =
image.GetSkImage()->makeNonTextureImage();
image = cc::PaintImageBuilder::WithProperties(image)
.set_image(std::move(non_texture_image), image.content_id())
.TakePaintImage();
}
// As we are using SharedImages to handle the image we are not using the skia
// internals to handle the image or the drawing in the canvas. The cache is
// modified to track the mailbox for the source texture, rather than a
// SkImage, to be able to import it into multiple contexts. Also the cached
// shared image is reused if possible (same context provider, same size) to
// reflect equivalent skia optimizations (SkImage pooling). See
// https://chromium-review.googlesource.com/c/chromium/src/+/1616978
// todo(juanmihd) remove the above comment once we are sure this is not needed
canvas->drawImage(image, 0, 0, &video_flags);

if (need_transform)
Expand Down

0 comments on commit 0cfb2d5

Please sign in to comment.