diff mbox

[libdrm,1/5] modetest: Make RGB565 pwetty too

Message ID 1366226285-13282-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä April 17, 2013, 7:18 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/modetest/buffers.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart April 18, 2013, 1:43 p.m. UTC | #1 
Hi Ville,

Thank you for the patch.

On Wednesday 17 April 2013 22:18:01 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/modetest/buffers.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
> index 5086381..6b117b4 100644
> --- a/tests/modetest/buffers.c
> +++ b/tests/modetest/buffers.c
> @@ -601,7 +601,7 @@ fill_smpte(const struct format_info *info, void
> *planes[3], unsigned int width, #define BLUE  0
> 
>  static void
> -make_pwetty(void *data, int width, int height, int stride)
> +make_pwetty(void *data, int width, int height, int stride, int rgb16)

What about passing the format 4cc instead ?

>  {
>  #ifdef HAVE_CAIRO
>  	cairo_surface_t *surface;
> @@ -609,7 +609,7 @@ make_pwetty(void *data, int width, int height, int
> stride) int x, y;
> 
>  	surface = cairo_image_surface_create_for_data(data,
> -						      CAIRO_FORMAT_ARGB32,
> +						      rgb16 ? CAIRO_FORMAT_RGB16_565 : 
CAIRO_FORMAT_ARGB32,
>  						      width, height,
>  						      stride);
>  	cr = cairo_create(surface);
> @@ -716,6 +716,7 @@ static void
>  fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem,
>  		 unsigned int width, unsigned int height, unsigned int stride)
>  {
> +	unsigned char *mem_base = mem;
>  	unsigned int x, y;
> 
>  	for (y = 0; y < height; ++y) {
> @@ -732,6 +733,8 @@ fill_tiles_rgb16(const struct rgb_info *rgb, unsigned
> char *mem, }
>  		mem += stride;
>  	}
> +
> +	make_pwetty(mem_base, width, height, stride, 1);
>  }
> 
>  static void
> @@ -777,7 +780,7 @@ fill_tiles_rgb32(const struct rgb_info *rgb, unsigned
> char *mem, mem += stride;
>  	}
> 
> -	make_pwetty(mem_base, width, height, stride);
> +	make_pwetty(mem_base, width, height, stride, 0);
>  }
> 
>  static void
Ville Syrjälä April 18, 2013, 2:06 p.m. UTC | #2 
On Thu, Apr 18, 2013 at 03:43:23PM +0200, Laurent Pinchart wrote:
> Hi Ville,
> 
> Thank you for the patch.
> 
> On Wednesday 17 April 2013 22:18:01 ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/modetest/buffers.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
> > index 5086381..6b117b4 100644
> > --- a/tests/modetest/buffers.c
> > +++ b/tests/modetest/buffers.c
> > @@ -601,7 +601,7 @@ fill_smpte(const struct format_info *info, void
> > *planes[3], unsigned int width, #define BLUE  0
> > 
> >  static void
> > -make_pwetty(void *data, int width, int height, int stride)
> > +make_pwetty(void *data, int width, int height, int stride, int rgb16)
> 
> What about passing the format 4cc instead ?

It's a bit more work since I need to pass the whole format_info
down from fill_tiles(). But I can make a v2 with that change.

> >  {
> >  #ifdef HAVE_CAIRO
> >  	cairo_surface_t *surface;
> > @@ -609,7 +609,7 @@ make_pwetty(void *data, int width, int height, int
> > stride) int x, y;
> > 
> >  	surface = cairo_image_surface_create_for_data(data,
> > -						      CAIRO_FORMAT_ARGB32,
> > +						      rgb16 ? CAIRO_FORMAT_RGB16_565 : 
> CAIRO_FORMAT_ARGB32,
> >  						      width, height,
> >  						      stride);
> >  	cr = cairo_create(surface);
> > @@ -716,6 +716,7 @@ static void
> >  fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem,
> >  		 unsigned int width, unsigned int height, unsigned int stride)
> >  {
> > +	unsigned char *mem_base = mem;
> >  	unsigned int x, y;
> > 
> >  	for (y = 0; y < height; ++y) {
> > @@ -732,6 +733,8 @@ fill_tiles_rgb16(const struct rgb_info *rgb, unsigned
> > char *mem, }
> >  		mem += stride;
> >  	}
> > +
> > +	make_pwetty(mem_base, width, height, stride, 1);
> >  }
> > 
> >  static void
> > @@ -777,7 +780,7 @@ fill_tiles_rgb32(const struct rgb_info *rgb, unsigned
> > char *mem, mem += stride;
> >  	}
> > 
> > -	make_pwetty(mem_base, width, height, stride);
> > +	make_pwetty(mem_base, width, height, stride, 0);
> >  }
> > 
> >  static void
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart April 18, 2013, 2:19 p.m. UTC | #3 
On Thursday 18 April 2013 17:06:57 Ville Syrjälä wrote:
> On Thu, Apr 18, 2013 at 03:43:23PM +0200, Laurent Pinchart wrote:
> > On Wednesday 17 April 2013 22:18:01 ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > 
> > >  tests/modetest/buffers.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
> > > index 5086381..6b117b4 100644
> > > --- a/tests/modetest/buffers.c
> > > +++ b/tests/modetest/buffers.c
> > > @@ -601,7 +601,7 @@ fill_smpte(const struct format_info *info, void
> > > *planes[3], unsigned int width, #define BLUE  0
> > > 
> > >  static void
> > > 
> > > -make_pwetty(void *data, int width, int height, int stride)
> > > +make_pwetty(void *data, int width, int height, int stride, int rgb16)
> > 
> > What about passing the format 4cc instead ?
> 
> It's a bit more work since I need to pass the whole format_info
> down from fill_tiles(). But I can make a v2 with that change.

It was just an idea to make the code more future-proof, in case we decide to 
make 24-bit formats pwetty at some point. Maybe a bpp value would be a good 
compromise ?
Ville Syrjälä April 18, 2013, 2:37 p.m. UTC | #4 
On Thu, Apr 18, 2013 at 04:19:30PM +0200, Laurent Pinchart wrote:
> On Thursday 18 April 2013 17:06:57 Ville Syrjälä wrote:
> > On Thu, Apr 18, 2013 at 03:43:23PM +0200, Laurent Pinchart wrote:
> > > On Wednesday 17 April 2013 22:18:01 ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > 
> > > >  tests/modetest/buffers.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
> > > > index 5086381..6b117b4 100644
> > > > --- a/tests/modetest/buffers.c
> > > > +++ b/tests/modetest/buffers.c
> > > > @@ -601,7 +601,7 @@ fill_smpte(const struct format_info *info, void
> > > > *planes[3], unsigned int width, #define BLUE  0
> > > > 
> > > >  static void
> > > > 
> > > > -make_pwetty(void *data, int width, int height, int stride)
> > > > +make_pwetty(void *data, int width, int height, int stride, int rgb16)
> > > 
> > > What about passing the format 4cc instead ?
> > 
> > It's a bit more work since I need to pass the whole format_info
> > down from fill_tiles(). But I can make a v2 with that change.
> 
> It was just an idea to make the code more future-proof, in case we decide to 
> make 24-bit formats pwetty at some point. Maybe a bpp value would be a good 
> compromise ?

I already implemented your original idea. Just doing a quick test now.

It's a better idea anyway since we can then check that the color
channels match what we're rendering. My original code just assumed
that it can render RGB565 into an XRGB1555 buffer for example. That
produces some rather ugly results in reality, so it's better to
limit it to the formats where the size/location of RGB channels match.
The order or RGB channels doesn't actually matter since the source
color is always white or black.
diff mbox

Patch

diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 5086381..6b117b4 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -601,7 +601,7 @@  fill_smpte(const struct format_info *info, void *planes[3], unsigned int width,
 #define BLUE  0
 
 static void
-make_pwetty(void *data, int width, int height, int stride)
+make_pwetty(void *data, int width, int height, int stride, int rgb16)
 {
 #ifdef HAVE_CAIRO
 	cairo_surface_t *surface;
@@ -609,7 +609,7 @@  make_pwetty(void *data, int width, int height, int stride)
 	int x, y;
 
 	surface = cairo_image_surface_create_for_data(data,
-						      CAIRO_FORMAT_ARGB32,
+						      rgb16 ? CAIRO_FORMAT_RGB16_565 : CAIRO_FORMAT_ARGB32,
 						      width, height,
 						      stride);
 	cr = cairo_create(surface);
@@ -716,6 +716,7 @@  static void
 fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem,
 		 unsigned int width, unsigned int height, unsigned int stride)
 {
+	unsigned char *mem_base = mem;
 	unsigned int x, y;
 
 	for (y = 0; y < height; ++y) {
@@ -732,6 +733,8 @@  fill_tiles_rgb16(const struct rgb_info *rgb, unsigned char *mem,
 		}
 		mem += stride;
 	}
+
+	make_pwetty(mem_base, width, height, stride, 1);
 }
 
 static void
@@ -777,7 +780,7 @@  fill_tiles_rgb32(const struct rgb_info *rgb, unsigned char *mem,
 		mem += stride;
 	}
 
-	make_pwetty(mem_base, width, height, stride);
+	make_pwetty(mem_base, width, height, stride, 0);
 }
 
 static void