Re: [PATCH 2/2] drm/vkms: Use a simpler composition function

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Pekka,

pekka.paalanen@haloniitty.fi wrote on Fri, 2 Feb 2024 10:55:22 +0200:

> On Thu, 01 Feb 2024 18:31:32 +0100
> Louis Chauvet <louis.chauvet@bootlin.com> wrote:
> 
> > Change the composition algorithm to iterate over pixels instead of lines.
> > It allows a simpler management of rotation and pixel access for complex formats.
> > 
> > This new algorithm allows read_pixel function to have access to x/y
> > coordinates and make it possible to read the correct thing in a block
> > when block_w and block_h are not 1.
> > The iteration pixel-by-pixel in the same method also allows a simpler
> > management of rotation with drm_rect_* helpers. This way it's not needed
> > anymore to have misterious switch-case distributed in multiple places.  
> 
> Hi,
> 
> there was a very good reason to write this code using lines:
> performance. Before lines, it was indeed operating on individual pixels.
> 
> Please, include performance measurements before and after this series
> to quantify the impact on the previously already supported pixel
> formats, particularly the 32-bit-per-pixel RGB variants.
> 
> VKMS will be used more and more in CI for userspace projects, and
> performance actually matters there.
> 
> I'm worrying that this performance degradation here is significant. I
> believe it is possible to keep blending with lines, if you add new line
> getters for reading from rotated, sub-sampled etc. images. That way you
> don't have to regress the most common formats' performance.

While I understand performance is important and should be taken into
account seriously, I cannot understand how broken testing could be
considered better. Fast but inaccurate will always be significantly
less attractive to my eyes.

I am in favor of making this working first, and then improving the code
for faster results. Maybe the line-driven approach can be dedicated to
"simpler" formats where more complex corner cases do not happen. But
for now I don't see the point in comparing performances between broken
and (hopefully) non broken implementations.

Thanks,
Miquèl