New issue
Advanced search Search tips

Issue 675977 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Clean up skia usage in skia/ext

Project Member Reported by reed@google.com, Dec 20 2016

Issue description

skia/ext has a lot of structure around supporting "native" drawing (e.g. core-graphics or GDI or cairo) into the same canvas as Skia. It does this by subclassing SkDevice. However, this is deprecated (and only works for raster backends, not PDF or pictures or GPU).

Goal: reduce / eliminate the usages of SkDevice subclasses in chrome.

This bottlenecks to only a few apis in skia/ext:
- GetPlatformDevice
- ScopedPlatformPaint
- GetBitmapContext

Approach: find alternate apis/techniques to allow chrome to stop using these.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a0a39813c19a9ea94360879ff7f1bd46adcf78c

commit 7a0a39813c19a9ea94360879ff7f1bd46adcf78c
Author: reed <reed@google.com>
Date: Wed Dec 21 17:13:44 2016

simplify scrollcanvas to always use shared code

note: scrollcanvas is only called by pepper

BUG= 675977 

Review-Url: https://codereview.chromium.org/2593803004
Cr-Commit-Position: refs/heads/master@{#440134}

[modify] https://crrev.com/7a0a39813c19a9ea94360879ff7f1bd46adcf78c/ui/gfx/blit.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47bc98adbbef3918db8d91fd0086f14582a0f2b3

commit 47bc98adbbef3918db8d91fd0086f14582a0f2b3
Author: reed <reed@google.com>
Date: Fri Dec 23 04:04:33 2016

clean-up and clarify CreatePlatformCanvas methods

Removed unused ones, and renamed for clarity some overloads that are platform specific (to make it possible to know which are used where).

BUG= 675977 
TBR=

Review-Url: https://codereview.chromium.org/2595023002
Cr-Commit-Position: refs/heads/master@{#440588}

[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/content/browser/compositor/software_output_device_win.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/bitmap_platform_device_cairo.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/bitmap_platform_device_mac.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/bitmap_platform_device_skia.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/bitmap_platform_device_win.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/platform_canvas.h
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/skia/ext/skia_utils_mac.mm
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/ui/gfx/blit_unittest.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/ui/surface/transport_dib_posix.cc
[modify] https://crrev.com/47bc98adbbef3918db8d91fd0086f14582a0f2b3/ui/surface/transport_dib_win.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ccc2155e4e16cf6b71707d97517a14625fc8c65f

commit ccc2155e4e16cf6b71707d97517a14625fc8c65f
Author: reed <reed@google.com>
Date: Sat Dec 24 13:40:46 2016

use skia cgimage->bitmap routine

In particular, this removes a dependency on PlatformCanvas ...

BUG= 675977 

Review-Url: https://codereview.chromium.org/2595113002
Cr-Commit-Position: refs/heads/master@{#440667}

[modify] https://crrev.com/ccc2155e4e16cf6b71707d97517a14625fc8c65f/skia/ext/skia_utils_mac.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b76da0d5e4f1e1aa75329a53a772de6e96b227b

commit 6b76da0d5e4f1e1aa75329a53a772de6e96b227b
Author: fmalita <fmalita@chromium.org>
Date: Tue Jan 03 15:30:06 2017

Remove PlatformCanvas::GetBitmapContext()

This API is only used from ~CanvasSkiaPaint(), and is similar in behavior
to ScopedPlatformPaint::GetNativeDrawingContext().

Refactor the only client to use the latter, get rid of the GetBitmapContext()
virtual and platform_device_foo stubs.

BUG= 675977 
R=reed@google.com
TBR=danakj@chromium.org

Review-Url: https://codereview.chromium.org/2607603004
Cr-Commit-Position: refs/heads/master@{#441127}

[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/BUILD.gn
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/bitmap_platform_device_mac.cc
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/bitmap_platform_device_mac.h
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/platform_canvas.cc
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/platform_canvas.h
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/platform_device.cc
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/skia/ext/platform_device.h
[delete] https://crrev.com/6fa5a21dd161d314265404d297e18f8ea292ef4f/skia/ext/platform_device_linux.cc
[delete] https://crrev.com/6fa5a21dd161d314265404d297e18f8ea292ef4f/skia/ext/platform_device_mac.cc
[delete] https://crrev.com/6fa5a21dd161d314265404d297e18f8ea292ef4f/skia/ext/platform_device_win.cc
[modify] https://crrev.com/6b76da0d5e4f1e1aa75329a53a772de6e96b227b/ui/gfx/canvas_paint_mac.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dfb527a33abfabb690802b64136156a867ccdbae

commit dfb527a33abfabb690802b64136156a867ccdbae
Author: fmalita <fmalita@chromium.org>
Date: Wed Jan 04 14:15:16 2017

Remove skia::ScopedPlatformPaint

This helper no longer provides any scoped/RAII functionalty, and can be
replaced with a GetNativeDrawingContext() function.

BUG= 675977 
R=reed@google.com
TBR=sky@chromium.org,danakj@chromium.org

Review-Url: https://codereview.chromium.org/2613583002
Cr-Commit-Position: refs/heads/master@{#441367}

[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/content/browser/compositor/software_output_device_win.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/bitmap_platform_device_cairo.h
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/bitmap_platform_device_mac_unittest.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/bitmap_platform_device_skia.h
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/bitmap_platform_device_win.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/bitmap_platform_device_win.h
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/platform_canvas.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/platform_canvas.h
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/platform_canvas_unittest.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/skia/ext/platform_device.h
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/ui/base/clipboard/clipboard_win.cc
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/ui/gfx/canvas_paint_mac.mm
[modify] https://crrev.com/dfb527a33abfabb690802b64136156a867ccdbae/ui/native_theme/native_theme_win.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6

commit 233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6
Author: fmalita <fmalita@chromium.org>
Date: Wed Jan 04 16:29:29 2017

Remove skia::SupportsPlatformPaint()

It is equivalent to checking the result of GetNativeDrawingContext(), so
do that instead, in NativeThemeWin::Paint().

Also remove an unused param of NativeThemeWin::PaintIndirect().

BUG= 675977 
R=reed@google.com

Review-Url: https://codereview.chromium.org/2610003002
Cr-Commit-Position: refs/heads/master@{#441388}

[modify] https://crrev.com/233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6/skia/ext/platform_canvas.cc
[modify] https://crrev.com/233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6/skia/ext/platform_canvas.h
[modify] https://crrev.com/233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6/ui/native_theme/native_theme_win.cc
[modify] https://crrev.com/233c886dc8f37fe7ba3b53f3c7e5e431cb150ff6/ui/native_theme/native_theme_win.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c5054aecae9bb99c972e7cca364053a99c4df39

commit 4c5054aecae9bb99c972e7cca364053a99c4df39
Author: reed <reed@google.com>
Date: Wed Jan 04 16:41:01 2017

don't need native-drawing-context for CanvasSkiaPaint

BUG= 675977 

Review-Url: https://codereview.chromium.org/2612023003
Cr-Commit-Position: refs/heads/master@{#441392}

[modify] https://crrev.com/4c5054aecae9bb99c972e7cca364053a99c4df39/ui/gfx/canvas_paint_mac.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35b5c4707ee990bd2280d3b43e2d9ea5a6dddcf4

commit 35b5c4707ee990bd2280d3b43e2d9ea5a6dddcf4
Author: reed <reed@google.com>
Date: Thu Jan 05 17:20:28 2017

don't need gfx::Canvas and its native context to convert HBITMAP

Because we can now hold onto the hbitmap for the life of the skia_bitmap, we save a copy over the prev. impl.

BUG= 675977 

Review-Url: https://codereview.chromium.org/2615683002
Cr-Commit-Position: refs/heads/master@{#441684}

[modify] https://crrev.com/35b5c4707ee990bd2280d3b43e2d9ea5a6dddcf4/ui/base/clipboard/clipboard_win.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf

commit ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf
Author: fmalita <fmalita@chromium.org>
Date: Thu Jan 05 21:13:56 2017

Delete Mac & Cairo skia::BitmapPlatformDevice impls

We are no longer performing native drawing on these platforms, use the
Skia placeholder instead.

BUG= 675977 
R=reed@google.com
TBR=

Review-Url: https://codereview.chromium.org/2611153002
Cr-Commit-Position: refs/heads/master@{#441750}

[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/skia/BUILD.gn
[delete] https://crrev.com/53ce627c8aa95544706c84a918170a8e53f5a20e/skia/ext/bitmap_platform_device.h
[delete] https://crrev.com/53ce627c8aa95544706c84a918170a8e53f5a20e/skia/ext/bitmap_platform_device_cairo.cc
[delete] https://crrev.com/53ce627c8aa95544706c84a918170a8e53f5a20e/skia/ext/bitmap_platform_device_cairo.h
[delete] https://crrev.com/53ce627c8aa95544706c84a918170a8e53f5a20e/skia/ext/bitmap_platform_device_mac.cc
[delete] https://crrev.com/53ce627c8aa95544706c84a918170a8e53f5a20e/skia/ext/bitmap_platform_device_mac.h
[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/skia/ext/bitmap_platform_device_skia.cc
[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/skia/ext/bitmap_platform_device_skia.h
[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/skia/ext/bitmap_platform_device_win.h
[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/skia/ext/platform_device.h
[modify] https://crrev.com/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf/ui/native_theme/native_theme_win.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f9a13711bef4107aeeb4e15ee296b917228c141

commit 0f9a13711bef4107aeeb4e15ee296b917228c141
Author: sky <sky@chromium.org>
Date: Fri Jan 06 00:09:14 2017

Revert of Delete Mac & Cairo skia::BitmapPlatformDevice impls (patchset #3 id:40001 of https://codereview.chromium.org/2611153002/ )

Reason for revert:
Revert in hopes of fix linux x64 bot: https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux%20x64/builds/15545

Original issue's description:
> Delete Mac & Cairo skia::BitmapPlatformDevice impls
>
> We are no longer performing native drawing on these platforms, use the
> Skia placeholder instead.
>
> BUG= 675977 
> R=reed@google.com
> TBR=
>
> Review-Url: https://codereview.chromium.org/2611153002
> Cr-Commit-Position: refs/heads/master@{#441750}
> Committed: https://chromium.googlesource.com/chromium/src/+/ccdceaed36d07ad5ab6d5e504ece0459a4aeaddf

TBR=reed@google.com,robertphillips@chromium.org,fmalita@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 675977 

Review-Url: https://codereview.chromium.org/2616003003
Cr-Commit-Position: refs/heads/master@{#441776}

[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/BUILD.gn
[add] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device.h
[add] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_cairo.cc
[add] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_cairo.h
[add] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_mac.cc
[add] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_mac.h
[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_skia.cc
[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_skia.h
[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/bitmap_platform_device_win.h
[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/skia/ext/platform_device.h
[modify] https://crrev.com/0f9a13711bef4107aeeb4e15ee296b917228c141/ui/native_theme/native_theme_win.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09884beef8d9c585d4debd606c394bbd68e44ad5

commit 09884beef8d9c585d4debd606c394bbd68e44ad5
Author: fmalita <fmalita@chromium.org>
Date: Fri Jan 06 20:13:38 2017

Delete Mac & Cairo skia::BitmapPlatformDevice impls

(reland with updated Linux .deb shared lib deps)

We are no longer performing native drawing on these platforms, use the
Skia placeholder instead.

BUG= 675977 
R=reed@google.com
TBR=

Review-Url: https://codereview.chromium.org/2611153002
Cr-Commit-Position: refs/heads/master@{#442023}

[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/chrome/installer/linux/debian/expected_deps_ia32_jessie
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/chrome/installer/linux/debian/expected_deps_ia32_wheezy
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/chrome/installer/linux/debian/expected_deps_x64_jessie
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/chrome/installer/linux/debian/expected_deps_x64_wheezy
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/skia/BUILD.gn
[delete] https://crrev.com/9d43cc2631f51968d3790e8709dde20bdedb5a6d/skia/ext/bitmap_platform_device.h
[delete] https://crrev.com/9d43cc2631f51968d3790e8709dde20bdedb5a6d/skia/ext/bitmap_platform_device_cairo.cc
[delete] https://crrev.com/9d43cc2631f51968d3790e8709dde20bdedb5a6d/skia/ext/bitmap_platform_device_cairo.h
[delete] https://crrev.com/9d43cc2631f51968d3790e8709dde20bdedb5a6d/skia/ext/bitmap_platform_device_mac.cc
[delete] https://crrev.com/9d43cc2631f51968d3790e8709dde20bdedb5a6d/skia/ext/bitmap_platform_device_mac.h
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/skia/ext/bitmap_platform_device_skia.cc
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/skia/ext/bitmap_platform_device_skia.h
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/skia/ext/bitmap_platform_device_win.h
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/skia/ext/platform_device.h
[modify] https://crrev.com/09884beef8d9c585d4debd606c394bbd68e44ad5/ui/native_theme/native_theme_win.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 6 2017

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/151280976aad5ebe2285cf1b446e98ea9635600c

commit 151280976aad5ebe2285cf1b446e98ea9635600c
Author: reed <reed@google.com>
Date: Fri Jan 06 20:37:26 2017

use SkSurface behind gfx::Canvas

Using PlatformCanvas (previously) always paid for the cost of the native graphics context (e.g. HDC or CGContext), but there are no callers of gfx::Canvas that use those facilities.

BUG= 675977 

Review-Url: https://codereview.chromium.org/2615493003
Cr-Commit-Position: refs/heads/master@{#442034}

[modify] https://crrev.com/151280976aad5ebe2285cf1b446e98ea9635600c/ui/gfx/canvas.cc
[modify] https://crrev.com/151280976aad5ebe2285cf1b446e98ea9635600c/ui/gfx/canvas.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 10 2017

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9ba16d598eecb848557551eb4e02e5b4ffceca2

commit a9ba16d598eecb848557551eb4e02e5b4ffceca2
Author: fmalita <fmalita@chromium.org>
Date: Fri Jan 13 20:13:43 2017

Remove unused SkDevice.h include in ppb_image_data_impl.cc

No reason for this include, plus we're about to make the header private
in Skia.

BUG= 675977 
R=reed@google.com,robertphillips@chromium.org
TBR=
NOTRY=true

Review-Url: https://codereview.chromium.org/2637453002
Cr-Commit-Position: refs/heads/master@{#443648}

[modify] https://crrev.com/a9ba16d598eecb848557551eb4e02e5b4ffceca2/content/renderer/pepper/ppb_image_data_impl.cc

Components: Internals>Skia
Status: Fixed (was: Untriaged)
SkDevice.h has been moved to Skia's src/core (is out of the public API) and code search finds no use of SkDevice.h anymore in Chromium. In addition, the PlatformCanvas class has been removed and replaced with SkCanvas.

Sign in to add a comment