Bug 191577

Summary: [cairo] BackingStore::incorporateUpdate should use CompositeCopy to support drawsBackground=false in general Cairo ports
Product: WebKit Reporter: Tomoki Imai <tomoki.imai>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, don.olmstead, webkit-bug-importer, zan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Test HTML which doesn't have background color.
none
Actual result of the testcase
none
Use CompositeCopy to update bitmap to overwrite existing surface
none
Use CompositeCopy to update bitmap to overwrite existing surface none

Description Tomoki Imai 2018-11-12 21:02:25 PST
Created attachment 354641 [details]
Test HTML which doesn't have background color.

I'm trying to implement a transparent background feature in Cairo port, and found that it works only in GTK+ port because of BackingStore::incorporateUpdate.

# How to reproduce

The other cairo ports doesn't have transparent background feature yet, but this is reproducable in GTK+ port by removing platform specific code.

1. Delete GTK+ specific code from BackingStore::incorporateUpdate
- https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/UIProcess/cairo/BackingStoreCairo.cpp?rev=227728#L96
2. Build WebKitGTK+ as usual
3. Run MiniBrowser with custom background color.
- $ Tools/Scripts/run-minibrowser --gtk --bg-color="rgba(0,0,0,0)"
4. Go to webpage which doesn't have background.
- You can use attached HTML file.

# Expected result

The webpage's background becomes transparent.

# Actual result

You see collapsed image because previous image are not cleared. (I'll attach image)

I'll post the patch.
Comment 1 Tomoki Imai 2018-11-12 21:06:08 PST
Created attachment 354644 [details]
Actual result of the testcase

At first, MiniBrowser opens WebKitGTK+ webpage. 
After that, I opened Attachment 354641 [details].
Comment 2 Tomoki Imai 2018-11-12 21:18:49 PST
Created attachment 354645 [details]
Use CompositeCopy to update bitmap to overwrite existing surface

We should set graphicsContext.setCompositeOperation(WebCore::CompositeCopy) before updating surface.
We should overwrite existing surface by the source even if it contains non opaque parts.
Comment 3 Tomoki Imai 2018-11-12 21:39:00 PST
Created attachment 354649 [details]
Use CompositeCopy to update bitmap to overwrite existing surface

I updated ChangeLog as the bugtitle doesn't match to bugzilla, and the style is different from the others.
Comment 4 Tomoki Imai 2018-11-12 21:45:22 PST
I guess we can get rid of GTK+ specific code from BackingStore::incorporateUpdate if user-defined background color is used in WebProcess side.
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp?rev=238057#L2817
Comment 5 Carlos Garcia Campos 2018-11-19 02:20:43 PST
Comment on attachment 354649 [details]
Use CompositeCopy to update bitmap to overwrite existing surface

Ok.
Comment 6 WebKit Commit Bot 2018-11-19 13:22:57 PST
Comment on attachment 354649 [details]
Use CompositeCopy to update bitmap to overwrite existing surface

Clearing flags on attachment: 354649

Committed r238379: <https://trac.webkit.org/changeset/238379>
Comment 7 WebKit Commit Bot 2018-11-19 13:22:59 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-11-19 13:23:32 PST
<rdar://problem/46173057>