search

linebender / piet

An abstraction for 2D graphics.
Apache License 2.0
1.25k stars 94 forks source link

Improve piet-coregraphics #528

Closed longmathemagician closed 1 year ago

longmathemagician commented 2 years ago

CoreGraphicsContext::draw_image_area() doesn't account for coregraphics' reversed y-axis, 086e8e613dfc951f869b6f6ebb9ebcba012ab25a fixes that by passing the cropped image to CoreGraphicsContext::draw_image() which does draw correctly.

xStrom commented 2 years ago

Please rebase on master to fix clippy errors and then also run cargo fmt.

longmathemagician commented 2 years ago

Added one last (hopefully) commit to this branch: 5df4eec2dbfe26fdad0c675c489210e33daea782 fixes capture_image_area for y-up contexts by tracking the y-direction in CoreGraphicsContext and translating the context's origin if necessary.

xStrom commented 2 years ago

I haven't looked into it deeper yet, but the test image is definitely broken now.

coregraphics-test-16-1 00 .

longmathemagician commented 2 years ago

Dang, this actually broke a lot of stuff that wasn't in my test case. Getting à la cart piet snapshot testing set up locally and then harmonizing y-direction handling across druid with and without bitmap caching (linebender/druid#2246) may take me a day or two.

If you want I can just shelve the commit, it's not likely to help anyone who isn't also using bitmap render caching of some form in druid.

xStrom commented 2 years ago

No rush, we can see what else comes up.

longmathemagician commented 2 years ago

Not thrilled about adding overhead in 6bbf3a18ad41255730ed699071210bab9899df5a but this is just about ready to go. Piet tests, standard druid windows, and bitmap-cached druid windows all draw properly. Still fighting a couple of coregraphics functions (mostly CGImageCreateWithImageInRect, which does not actually strip excess data when cropping so capture_image_area is still bugged if you want to access raw image data). Mixing y-up and y-down context content ``works'' but is approximately as inadvisable now as before.

I made a handful of changes to test pattern 16 in be1da570da12fca60bb23b4463d5cf0401e31805 to troubleshoot y-flip issues. A bunch of the test images fail out of the box on my system (macOS 12.5, perhaps coregraphics itself has changed) so I hesitate to make a PR to the snapshot repo. That image now looks like this: test_image

longmathemagician commented 2 years ago

Oops, I was using as_ref in another branch and didn't catch that with clippy. Some form of unwrapper is needed for sharing assets across contexts with differing y-directions, so I just made as_ref public and axed pub fn copy_image since it didn't actually copy yet (a weirdly difficult thing for coregraphics it seems).

For the snapshots, I don't know if the standard here is to have more comprehensive tests or faster/simpler ones. If simpler is better I can just drop be1da570da12fca60bb23b4463d5cf0401e31805.

longmathemagician commented 2 years ago

Pulled the changes to test image 16, I'll throw that in a separate PR. Took care of the last issue I had with coregraphics in f5abbb15391aa585440f6c5c0725c2251a4f432f, as long as I didn't break anything else in the process this branch is ready for review / merge.

jneem commented 1 year ago

@longmathemagician can I go ahead and merge this? Or is it waiting on the updated test image 16?

longmathemagician commented 1 year ago

@jneem you're good to merge it, thank you. There are still a lot of issues with piet-coregraphics I never got around to tackling but this is at least an improvement over the current state.