[vlc-devel] [PATCH v2 01/19] display: specify if vout_display_PlacePicture() alignment is top/bottom-left

[Alexandre Janniaux]

Hi,

On Wed, Aug 26, 2020 at 10:03:00AM +0200, Steve Lhomme wrote:
> On 2020-08-26 9:26, Romain Vimont wrote:
> > On Wed, Aug 26, 2020 at 09:08:39AM +0200, Steve Lhomme wrote:
> > > On 2020-08-26 8:50, Steve Lhomme wrote:
> > > > On 2020-08-25 17:17, Romain Vimont wrote:
> > > > > On Tue, Aug 25, 2020 at 04:20:14PM +0200, Steve Lhomme wrote:
> > > > > > For OpenGL the picture alignment must be computed from the
> > > > > > bottom-left, whereas
> > > > > > all other modules align from top-left.
> > > > >
> > > > > IMO, the core should not be aware of any "internal" vflip by the vout.
> > > >
> > > > I'm not sure what you mean by "be aware". The
> > > > vout_display_PlacePicture() function is in the core. But the core itself
> > > > doesn't know anything about the orientation/origin of the display. This
> > > > state is private to the display module(s).
> > > >
> > > > > It should just request the alignment it wants, and the vout handles it.
> > > >
> > > > This is what is done and is not changed.
> > > >
> > > > > > This was handled internally by OpenGL modules, but for center vertical
> > > > > > alignment on odd vertical dimensions, it may round to the same
> > > > > > value regardless
> > > > > > of the display coordinate system, which is incorrect.
> > > > >
> > > > > What is rounded to the same value in that case?
> > > >
> > > > You have a 1000x4 video. You want it aligned to the bottom in a 1000x7
> > > > window.
> > > > In the core/top-left-origin case, the placement is (0,3).
> > > > In the opengl/bottom-left-origin, the placement is (0,4).
> > >
> > > Uh, sorry. The rounding issue is only when using the center alignment.
> > >
> > > So a 1000x4 video in a 1000x7 video, with the current code we have:
> > > - in the core/top-left-origin case, the centered placement is (0,1).
> > > - in the opengl/bottom-left-origin, the centered placement is (0,1).
> > >
> > > This is what the core expects to be displayed
> > > ^
> > > |
> > > |
> > > |xxxxxxxxxxx
> > > |xxxxxxxxxxx
> > > |xxxxxxxxxxx
> > > |xxxxxxxxxxx
> > > |
> > > +----------->
> > >
> > >
> > > This is what OpenGL will display with the rounding mismatch
> > > +----------->
> > > |
> > > |xxxxxxxxxxx
> > > |xxxxxxxxxxx
> > > |xxxxxxxxxxx
> > > |
> > > |
> > > |
> > > v
> >
> > Sorry, I fail to see why.
>
> Do you fail to see that the pixels are not where the core expects them to be
> ? The position of the xxx are supposed to be exactly the same.
>
> > The fact that OpenGL stores pictures upside-down is an internal detail,
> > and everything (including placements) should be vflipped internally by
> > the OpenGL vout.
> >
> > In particular, if the OpenGL module calls vout_display_PlacePicture(),
> > it should vflip the placement too (the vout_display_place_t rectangle),
> > so in the end it will be correct.
> >
> > Even if the OpenGL vout stored the image rotated by 90 degrees (or any
> > other inversible transformation), as long as the placement is correctly
> > converted, it would also work.
>
> The display module has to do what the core tells it to do. If the core tells
> it to align at the center vertically, it just does that. The core assumes it
> does what it's asked. And when the core translates mouse positions according
> to video coordinates, it expects the video to be in a certain place. In this
> case the opengl module(s) do not render the video where they are expected to
> and the mouse coordinate can be off by one.

You should probably add documentation on the alignment enum
to specify how the rounding should take place, because it's
not immediate that a center alignment implies a lower value
rounding in term of window (thus mouse event) coordinates.

Otherwise, there's probably no «expected» behaviour required
here.

Regards,
--
Alexandre Janniaux
Videolabs