linebender / piet

An abstraction for 2D graphics.
Apache License 2.0
1.23k stars 95 forks source link

Modify capture_image_area to work with druid direct2d surfaces #526

Closed longmathemagician closed 1 year ago

longmathemagician commented 2 years ago

Using capture_image_area on a druid paint context currently fails on windows/direct2d for the following reasons:

1) Druid paint contexts have layer masks applied to the render context, which causes the call to ID2D1Bitmap::CopyFromRenderTarget to fail.

2) Druid's internal surface format is BGRA and piet_direct2d::D2DRenderContext::create_blank_bitmap hardcoded RGBA, which also causes the call to ID2D1Bitmap::CopyFromRenderTarget to fail.

This PR fixes the above two issues so that capture_image_area works inside druid widget paint loops. The caveat is that as the returned image format now matches the druid surface format, you can receive captured images which are not RGBA. That's not an issue for my uses as I'm drawing the captured image back to the paint context, but it probably could be an issue for some.

cmyr commented 1 year ago

Sorry about the delay here!

I don't currently have access to a windows machine, but the linked docs do clearly state that you need to pop clips before calling this method. I am slightly worried, however, that this could have unexpected consequences? To my eye, it feels like the intention of this method should be to capture the current state of the context, including any masking? Although maybe I'm misunderstanding, if removing the clip layers doesn't in any way change the visual appearance, then my concern is irrelevant.

longmathemagician commented 1 year ago

Hey no worries timing wise, my forks of druid and piet & PRs are a bit of a mess at the moment anyway since I've primarily been focused on supporting a single application.

Regarding your concern, that's certainly valid. My use of masks in piet is minimal so I haven't noticed any issues with this, but I can throw together a test case later and see how it plays out.

Unfortunately I'm not sure if there's a simple way to capture a region of a dxgi surface render target without popping off the layers. I suspect an alternate method would involve duplicating the surface's command list (which is problematic in its own ways). If you (or anyone else 'round here) have an alternate path that looks viable I'm happy to try implementing it, but I'm not familiar enough with direct2d to have any concrete fix for this in mind.

longmathemagician commented 1 year ago

@cmyr Sorry for the delay. I finally had time to look into this a bit more. It does capture the entire (unclipped) canvas, e.g. if you clip to a centered circle and capture the canvas you receive a bitmap of the entire canvas and not one with just the centered circle filled in and the rest transparent. This capture_image_area captures the canvas exactly as it is rendered on screen (at least in all of the tests I did), with all layers essentially flattened.

IMO this is currently fine for handling a PietImage*. When you want just a portion of the captured image, clip (or just leave clipped, this restores existing masks) the context before the draw call and it works exactly as expected. This is consistent with how capture_image_area works in piet-cairo and piet-coregraphics.

But, if what this should be doing is instead capturing only the intersection of the union of layer masks and the provided source rect then it'll need to be fixed.

Doing that may or may not be tricky. A potential approach is using ID2D1DeviceContext::GetTarget to get an ID2D1Image (rather than an ID2D1Bitmap from ID2D1Bitmap::CopyFromRenderTarget) and then drawing it to the target_bitmap. Another possibility is to apply the cached layers+masks to either the target_bitmap's rendertarget or an intermediate to ensure that only the desired region is painted. Either way it will probably incur an additional performance penalty.

Both of those approaches ended in failure in the short amount of time I had to try them. They're probably viable paths though, either if I spend more time troubleshooting or if someone who is better with Direct2D wants to take over.

* There is a conversation to be had concerning handling of a PietImage with regard to exporting it out of the active context or encoding them to some standard image format. Currently this doesn't really work, the image ends up in one of a variety of pixel formats depending on the platform and how you created your context (i.e. whether you're using druid+piet or just piet), not to mention that IIRC an ID2D1Bitmap lives in GPU memory. From #481 I take it this is not a high priority task.

capture_image_area in piet-coregraphics under druid is problematic, linebender/druid#2246 makes it functional but it's still pretty bad.

longmathemagician commented 1 year ago

@jneem unless @cmyr disagrees this can be merged as well.

cmyr commented 1 year ago

I think this is probably an improvement! 💁