Fix rendering of source clipped images with the MCU renderer

[tronical]

The slide puzzle makes use of the source-clip-* properties and that unveiled three issues:

• For the scale factor from image space to physical pixel space (sx/sy),
  use the source clip size, as we're only drawing those
  pixels. That means that the call sites of draw_image_impl need to pass the
  correct original image size if there is no source clip.
• Similarly, the scaled_clip is in image coordinates, but needs to be relative to the source-clip-x/y
• The geometry for the target rectangle on screen
  must originate at (0, 0) so that the apllication of
  sx/sy only changes the size.

[tronical]

This could also be written differently, if you prefer: Create the rectangle from offset as origin and size directly calculated by taking clipped_src.size scaled with sx/sy. Maybe that's clearer?

[ogoffart]

I like the part in draw_clipped_image and draw_image. It makes sense to have the source_clip limited to the actual size of the image.

However, the merging with the actual clip seems off.

There are 3 coordinate system to take care of:

• the logical one
• the physical one
• the image one

scaled_clip is transforming the current_state.clip which is in logical coordinate to the image coordinate.
But then you add a translation by the source_clip.origin that make sense because the image should actually be translated by that amount. But that is not in any of the coordinate system listed, this is a new one: the translated image. So clipped_src will then be in that referential too.

The geometry then is in physical coordinate. But it should match to the location of the clipped rect of the image to the physical screen. In particular, you are dropping the clipped part from the clip. (making it potentially rendering into area that should be clipped away)

I think one proper fix here is to translate t.rect.intersection(&source_clip) by -source_clip.origin to move the image coordinate into the translated image coordinate.
the actual_x and actual_y also need to be adjusted.

In other word, i think we should apply the transformation by source_clip.origin on the image coordinate

I really would love to see some rendering tests that render a small scene into a .png so we can do image comparison.

[ogoffart]

is that not source_clip.origin.to_vector() ?

(there is also the `euclid::vec2' helper function to simplify that kind of code.)

[tronical]

Yes, that's true. Modulo the type tag.

[ogoffart]

This ignores clipped_src.origin and i think it is wrong if the image is clipped. You can try the x button in the printer demo when scrolled partially out of the ListView. Does it still looks alright?

[tronical]

You're right, the scaled clip could cover the top-left of the Image geometry and thus should adjust clipped_src.origin (in image space through the intersection call).

[tronical]

But that is not in any of the coordinate system listed, this is a new one: the translated image.

You're right, this approach is introducing a fourth coordinate system, and that's not helping to make the code easier to understand.

I think one proper fix here is to translate t.rect.intersection(&source_clip) by -source_clip.origin to move the image coordinate into the translated image coordinate.
the actual_x and actual_y also need to be adjusted.

You're right, that's simpler and appears to work as well (and certainly won't suffer from the missing origin breaking the renderer's clip).

I'll add comments to explain the steps (and will rename r ;)

[tronical]

But that is not in any of the coordinate system listed, this is a new one: the translated image.

On second thought, I think translating scaled_clip by source_clip.origin is better than translating t.rect.intersection(&source_clip) by -source_clip.origin. The former keeps everything in original image space, the latter introduces a translated image space.

Edit: Scratch that, Olivier's approach is the better one :)