Message ID | 1366226285-13282-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
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 ?
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 --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