compositor-headless: Report a more realistic physical size

Submitted by Johan Helsing on April 12, 2018, 7:31 a.m.

Details

Message ID 20180412073148.17453-1-johan.helsing@qt.io
State New
Series "compositor-headless: Report a more realistic physical size"
Headers show

Commit Message

Johan Helsing April 12, 2018, 7:31 a.m.
Some clients rely on the physical size to determine the physical DPI. With the
previous implementation, we would report 1px==1mm, which is a DPI of 25.4,
which is incredibly low.

The problem is solved by setting a physical size so the DPI is close to 72
instead. If the output is scaled, the DPI is set to the corresponding multiple
of 72.

This makes the headless backend more usable for automated testing of DPI
sensitive functionality such as point sized fonts.

Signed-off-by: Johan Klokkhammer Helsing <johan.helsing@qt.io>
---
 libweston/compositor-headless.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
index 9307a36a..9f1a1a72 100644
--- a/libweston/compositor-headless.c
+++ b/libweston/compositor-headless.c
@@ -180,12 +180,19 @@  err_malloc:
 	return -1;
 }
 
+static int
+physical_size_for_dpi(int pixels, int dpi)
+{
+	static const float mm_per_inch = 25.4;
+	return pixels * mm_per_inch / dpi;
+}
+
 static int
 headless_output_set_size(struct weston_output *base,
 			 int width, int height)
 {
 	struct headless_output *output = to_headless_output(base);
-	int output_width, output_height;
+	int output_width, output_height, output_dpi;
 
 	/* We can only be called once. */
 	assert(!output->base.current_mode);
@@ -207,9 +214,10 @@  headless_output_set_size(struct weston_output *base,
 	output->base.make = "weston";
 	output->base.model = "headless";
 
-	/* XXX: Calculate proper size. */
-	output->base.mm_width = width;
-	output->base.mm_height = height;
+	/* XXX: Make this configurable */
+	output_dpi = 72 * output->base.scale;
+	output->base.mm_width = physical_size_for_dpi(output_width, output_dpi);
+	output->base.mm_height = physical_size_for_dpi(output_height, output_dpi);
 
 	output->base.start_repaint_loop = headless_output_start_repaint_loop;
 	output->base.repaint = headless_output_repaint;

Comments

Pekka Paalanen July 2, 2018, 2:22 p.m.
On Thu, 12 Apr 2018 09:31:48 +0200
Johan Klokkhammer Helsing <johan.helsing@qt.io> wrote:

> Some clients rely on the physical size to determine the physical DPI. With the
> previous implementation, we would report 1px==1mm, which is a DPI of 25.4,
> which is incredibly low.
> 
> The problem is solved by setting a physical size so the DPI is close to 72
> instead. If the output is scaled, the DPI is set to the corresponding multiple
> of 72.
> 
> This makes the headless backend more usable for automated testing of DPI
> sensitive functionality such as point sized fonts.
> 
> Signed-off-by: Johan Klokkhammer Helsing <johan.helsing@qt.io>
> ---
>  libweston/compositor-headless.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)

Hi,

this is a good idea, but could you rebase this patch to master?


Thanks,
pq

> 
> diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
> index 9307a36a..9f1a1a72 100644
> --- a/libweston/compositor-headless.c
> +++ b/libweston/compositor-headless.c
> @@ -180,12 +180,19 @@ err_malloc:
>  	return -1;
>  }
>  
> +static int
> +physical_size_for_dpi(int pixels, int dpi)
> +{
> +	static const float mm_per_inch = 25.4;
> +	return pixels * mm_per_inch / dpi;
> +}
> +
>  static int
>  headless_output_set_size(struct weston_output *base,
>  			 int width, int height)
>  {
>  	struct headless_output *output = to_headless_output(base);
> -	int output_width, output_height;
> +	int output_width, output_height, output_dpi;
>  
>  	/* We can only be called once. */
>  	assert(!output->base.current_mode);
> @@ -207,9 +214,10 @@ headless_output_set_size(struct weston_output *base,
>  	output->base.make = "weston";
>  	output->base.model = "headless";
>  
> -	/* XXX: Calculate proper size. */
> -	output->base.mm_width = width;
> -	output->base.mm_height = height;
> +	/* XXX: Make this configurable */
> +	output_dpi = 72 * output->base.scale;
> +	output->base.mm_width = physical_size_for_dpi(output_width, output_dpi);
> +	output->base.mm_height = physical_size_for_dpi(output_height, output_dpi);
>  
>  	output->base.start_repaint_loop = headless_output_start_repaint_loop;
>  	output->base.repaint = headless_output_repaint;
Simon Ser July 2, 2018, 2:29 p.m.
> On Thu, 12 Apr 2018 09:31:48 +0200
> Johan Klokkhammer Helsing <johan.helsing@qt.io> wrote:
>
> > Some clients rely on the physical size to determine the physical DPI. With the
> > previous implementation, we would report 1px==1mm, which is a DPI of 25.4,
> > which is incredibly low.
> >
> > The problem is solved by setting a physical size so the DPI is close to 72
> > instead. If the output is scaled, the DPI is set to the corresponding multiple
> > of 72.
> >
> > This makes the headless backend more usable for automated testing of DPI
> > sensitive functionality such as point sized fonts.
> >
> > Signed-off-by: Johan Klokkhammer Helsing <johan.helsing@qt.io>
> > ---
> >  libweston/compositor-headless.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
>
> Hi,
>
> this is a good idea, but could you rebase this patch to master?
>
>
> Thanks,
> pq

Hi,

Would it make sense to change the protocol to allow compositors to send a zero
physical size in case it isn't relevant?

> >
> > diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
> > index 9307a36a..9f1a1a72 100644
> > --- a/libweston/compositor-headless.c
> > +++ b/libweston/compositor-headless.c
> > @@ -180,12 +180,19 @@ err_malloc:
> >  	return -1;
> >  }
> >
> > +static int
> > +physical_size_for_dpi(int pixels, int dpi)
> > +{
> > +	static const float mm_per_inch = 25.4;
> > +	return pixels * mm_per_inch / dpi;
> > +}
> > +
> >  static int
> >  headless_output_set_size(struct weston_output *base,
> >  			 int width, int height)
> >  {
> >  	struct headless_output *output = to_headless_output(base);
> > -	int output_width, output_height;
> > +	int output_width, output_height, output_dpi;
> >
> >  	/* We can only be called once. */
> >  	assert(!output->base.current_mode);
> > @@ -207,9 +214,10 @@ headless_output_set_size(struct weston_output *base,
> >  	output->base.make = "weston";
> >  	output->base.model = "headless";
> >
> > -	/* XXX: Calculate proper size. */
> > -	output->base.mm_width = width;
> > -	output->base.mm_height = height;
> > +	/* XXX: Make this configurable */
> > +	output_dpi = 72 * output->base.scale;
> > +	output->base.mm_width = physical_size_for_dpi(output_width, output_dpi);
> > +	output->base.mm_height = physical_size_for_dpi(output_height, output_dpi);
> >
> >  	output->base.start_repaint_loop = headless_output_start_repaint_loop;
> >  	output->base.repaint = headless_output_repaint;
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen July 3, 2018, 7:22 a.m.
On Mon, 02 Jul 2018 10:29:48 -0400
Simon Ser <contact@emersion.fr> wrote:

> > On Thu, 12 Apr 2018 09:31:48 +0200
> > Johan Klokkhammer Helsing <johan.helsing@qt.io> wrote:
> >  
> > > Some clients rely on the physical size to determine the physical DPI. With the
> > > previous implementation, we would report 1px==1mm, which is a DPI of 25.4,
> > > which is incredibly low.
> > >
> > > The problem is solved by setting a physical size so the DPI is close to 72
> > > instead. If the output is scaled, the DPI is set to the corresponding multiple
> > > of 72.
> > >
> > > This makes the headless backend more usable for automated testing of DPI
> > > sensitive functionality such as point sized fonts.
> > >
> > > Signed-off-by: Johan Klokkhammer Helsing <johan.helsing@qt.io>
> > > ---
> > >  libweston/compositor-headless.c | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)  
> >
> > Hi,
> >
> > this is a good idea, but could you rebase this patch to master?
> >
> >
> > Thanks,
> > pq  
> 
> Hi,
> 
> Would it make sense to change the protocol to allow compositors to send a zero
> physical size in case it isn't relevant?

I think it's often full of lies anyway, because usually the reported
numbers come directly from EDID. I've heard rumours of something like
width=16 height=9 (mm? cm?) etc. when the physical size is unknowable
(e.g. a projector) but they still want to send the aspect ratio.

If existing toolkits can cope with it, zero would be fine. Whether they
do, I really don't know.

Something about aspect ratio might need to be taken into account
though. We've never really had non-square pixels, so the assumption of
square pixels is probably used throughout the stack. If output
information implied non-square pixels, does it mean client pixels will
be shown equally non-square or are they scaled assuming they are
originally square?

OTOH, headless reporting a sensible fake size for a fake output is
still a good idea in my opinion.


Thanks,
pq