Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Fill image context with background color if image node is opaque - #668

Merged
appleguy merged 2 commits into
facebookarchive:masterfrom
Adlai-Holler:imagenode-opaque-bg
Sep 20, 2015
Merged

Fill image context with background color if image node is opaque#668
appleguy merged 2 commits into
facebookarchive:masterfrom
Adlai-Holler:imagenode-opaque-bg

Conversation

@Adlai-Holler

Copy link
Copy Markdown
Contributor

This way you can get a performance boost by marking your image nodes as opaque. Before this PR, no matter what the background color, opaque image nodes with partially-transparent images always show a black background.

Comment thread AsyncDisplayKit/ASImageNode.mm Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a more efficient method to do this. Instead of creating a path and requiring CG to understand the shape of the path (which it may be able to determine is a rectangle, and short-circuit...but more likely, is not able to) — let's just call CGContextFillRect() :).

@appleguy

Copy link
Copy Markdown
Contributor

Great find, thanks! Did you discover this by 1) using an image that is actually transparent in areas, 2) over a background that is a solid non-black color? Just curious. Opaque drawing is great, not just because of the GPU compositing savings — but the solid 25% memory savings :) (helps both allocation time, and obviously footprint)

Sorry for the delay in reviewing. Naturally, this is close to merge-ready, but I'd like to see it changed to use the most direct CG method first.

@Adlai-Holler

Copy link
Copy Markdown
Contributor Author

@appleguy Yeah, both actually! Transparent image over white background (a little compose icon in a table cell)

Also thanks for helping me find a new function today ^^

EDIT: Ruby gem server gave us a 500

@appleguy

Copy link
Copy Markdown
Contributor

Retried now-/will see the result in the morning. Thanks for the quick turnaround!

Really impressed to see the diff for the collection view issue, and excited to look at that too. Thanks so much for jumping in with fix-guns blazing!!

@Adlai-Holler

Copy link
Copy Markdown
Contributor Author

Thanks man! I have a lot of respect for your work so I'm glad to help out!

@appleguy

Copy link
Copy Markdown
Contributor

@adly-holler haha, thanks!! I wonder if we need to check for alpha == 1.0 here on the color. I guess theoretically we should, although it is probably a fine behavior if your partial-alpha color gets drawn into a non-alpha backing store (it should theoretically just make it opaque?)

appleguy added a commit that referenced this pull request Sep 20, 2015
Fill image context with background color if image node is opaque
@appleguy
appleguy merged commit aee6709 into facebookarchive:master Sep 20, 2015
@Adlai-Holler

Copy link
Copy Markdown
Contributor Author

Good point. Testing in a Playground shows that it premultiplies the color components, so it renders as if it were blended onto a black background, which I'd say is reasonable. So if you use transparent blue as the background color for an opaque node you'll see a dark blue background.

@Adlai-Holler
Adlai-Holler deleted the imagenode-opaque-bg branch September 6, 2016 02:32
peter-iakovlev pushed a commit to peter-iakovlev/AsyncDisplayKit that referenced this pull request Jan 9, 2018
* fix SIMULATE_WEB_RESPONSE not imported facebookarchive#449

* Fix logging message for new api
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants