Override default DrawTarget::fill_solid implementation

[rfuest]

This PR overrides the default implementation of DrawTarget::fill_solid with an optimized implementation.

In my application this caused a 90% reduction in drawing time for one special case where most of the drawn rectangle was overlapping the display border.

[almindor]

This PR overrides the default implementation of DrawTarget::fill_solid with an optimized implementation.

In my application this caused a 90% reduction in drawing time for one special case where most of the drawn rectangle was overlapping the display border.

This is odd as fill_solid calls fill_contiguous which should at this point be doing pretty much the same thing. Am I missing something. Can you provide the example?

[rfuest]

fill_contiguous doesn't clip the area rectangle to the display bounding box.

[almindor]

fill_contiguous doesn't clip the area rectangle to the display bounding box.

Hmm I think it should but I also think this means both are wrong right now. The bounding box is defined from the sizes in new which are expected to be display size. That means that fill_solid won't be able to use the remaining 80x240 pixels of the off-screen framebuffer portion.

I think the proper solution is to bound to the framebuffer size inside fill_contiguous?

[rfuest]

Hmm I think it should but I also think this means both are wrong right now. The bounding box is defined from the sizes in new which are expected to be display size. That means that fill_solid won't be able to use the remaining 80x240 pixels of the off-screen framebuffer portion.

Maybe this should be configurable in some way? If the off-screen portion of the framebuffer is never used drawing outside the visible area isn't necessary and only takes additional time.

I think the proper solution is to bound to the framebuffer size inside fill_contiguous?

Implementing proper clipping in fill_contiguous is more complex, because it needs to be able to skip the parts of the colors iterator that are outside the drawn area. fill_solid doesn't need this additional complexity, because all pixels are the same, and will therefore be faster.

[almindor]

Maybe this should be configurable in some way? If the off-screen portion of the framebuffer is never used drawing outside the visible area isn't necessary and only takes additional time.

Not sure if that makes sense. If you're drawing outside the display size (but still within framebuffer) you'd be doing it on purpose no? To say.. scroll into it later?

Implementing proper clipping in fill_contiguous is more complex, because it needs to be able to skip the parts of the colors iterator that are outside the drawn area. fill_solid doesn't need this additional complexity, because all pixels are the same, and will therefore be faster.

Not really. Fill contiguous is 100% same from ST7789 perspective. There are only 2 differences to fill_solid from this driver's perspective:

1. The colors can differ (which has NO impact whatsoever it's just different u16 in the pipe)
2. The iter can stop sooner (which has no impact either because the setup allows doing a window bigger than data)

The window (clip area of data to be drawn) is set to the area rect in both cases. I'm actually unsure what happens if the window definition is out of bounds for the frame buffer atm. tbh.

[rfuest]

Not sure if that makes sense. If you're drawing outside the display size (but still within framebuffer) you'd be doing it on purpose no? To say.. scroll into it later?

Not in all cases. If you, for example, want to draw an animation that moves a rectangle from outside the frame into view you could just animate the position and not the size. In the first frames most of the rectangle would overlap the visible area.

1. The colors can differ (which has NO impact whatsoever it's just different u16 in the pipe)

It has an impact as soon as you start clipping.

2. The iter can stop sooner (which has no impact either because the setup allows doing a window bigger than data)

If you don't clip at the bottom but on the right side you need to skip some pixels after each row, which isn't handled correctly by just setting the update window.

[almindor]

If you don't clip at the bottom but on the right side you need to skip some pixels after each row, which isn't handled correctly by just setting the update window.

You'd have to clip this way only I guess. Not sure about the performance impact of that tho.

Given this your change makes sense, I'm still worried about the extra 80x240 framebuffer space being un fill_solid able. That to me seems like a bug.

Imagine you wanted to draw a rect there and then scroll it, the way the clipping is done now it won't be allowed at all. I think we should clip to the full 320x240 frame buffer instead.

[rfuest]

You'd have to clip this way only I guess. Not sure about the performance impact of that tho.

Yes, you would. There is a better way to do clipping included e-g, but it isn't available publicly yet.

Given this your change makes sense, I'm still worried about the extra 80x240 framebuffer space being un fill_solid able. That to me seems like a bug.

Imagine you wanted to draw a rect there and then scroll it, the way the clipping is done now it won't be allowed at all. I think we should clip to the full 320x240 frame buffer instead.

OK, I'll change the implementation.

[almindor]

You'd have to clip this way only I guess. Not sure about the performance impact of that tho.

Yes, you would. There is a better way to do clipping included e-g, but it isn't available publicly yet.

Given this your change makes sense, I'm still worried about the extra 80x240 framebuffer space being un fill_solid able. That to me seems like a bug. Imagine you wanted to draw a rect there and then scroll it, the way the clipping is done now it won't be allowed at all. I think we should clip to the full 320x240 frame buffer instead.

OK, I'll change the implementation.

Thanks, I'll run some tests later today or tomorrow and merge after.

[almindor]

Worked nicely, thanks!