[prev in list] [next in list] [prev in thread] [next in thread]
List: cairo
Subject: [cairo] RGB16_565 - a diff that fixes png writing.
From: David Hill <David.Hill () Sun ! COM>
Date: 2007-05-18 20:25:40
Message-ID: 464E0BC4.6050609 () Sun ! COM
[Download RAW message or body]
In my quest to restore 565....
I have a diff attached that shows my current proposed changes to the
tree. They restore CAIRO_FORMAT_RGB16_565 to cairo.h and remove it from
deprecated.
The rest of the diff is some corrections for cairo-image-surface.c and
then the changes needed in cairo-png.c to support writing a png from a
565 surface. I ran the tests and got the same 13 failures I had before I
started changing things.
Questions and issues....
0) It occurred to me that 565 should be conditionally supported. There
are only a few places in Cairo that would need an #ifdef to make that
happen. But then it would have to be added to configure....
1) What to do about the Xlib support. I am not sure what to do here,
(nor is it my primary platform). To me, I don't see any reason to
support it, but then again, I don't have easy access to a Linux port on
a PDA that may have a 565 visual. Even if X has a visual that does
support it, you would need a query for that visual and code to check
against the users requested surface type. I think it is reasonable to
have the Xlib backend not support CAIRO_FORMAT_RGB16_565, much the same
way as the other backends only support selective surfaces. I just an not
sure how to code this in a manner that the people that care about the
xlib backend are going to accept. If left up to me, I would put in an
assert which is what some of the other backends do.
2) #define CAIRO_FORMAT_VALID(format) ((format) <= CAIRO_FORMAT_RGB16_565)
This macro is currently only used in cairo-image-surface.c &
cairo-xlib-surface.c. Given what I said above, perhaps this macro needs
to be backend sensitive ?
3) Testing, I tested the png support with a hackup program (mytest.c
attached). If you tweak the app, it will create surfaces for 565, RGB
and ARGB and all of them produced valid png files. What other paths are
needed here.
So... what else guys ?
Dave Hill
["pngdiff.txt" (text/plain)]
diff --git a/src/cairo-deprecated.h b/src/cairo-deprecated.h
index 77523c4..7d963ca 100644
--- a/src/cairo-deprecated.h
+++ b/src/cairo-deprecated.h
@@ -36,20 +36,6 @@
#ifndef CAIRO_DEPRECATED_H
#define CAIRO_DEPRECATED_H
-/* The CAIRO_FORMAT_RGB16_565 value was added in cairo 1.2.0 as part
- * of fixing cairo's xlib backend to work with X servers advertising a
- * 16-bit, 565 visual. But as it turned out, adding this format to
- * #cairo_format_t was not necessary, and was a mistake, (cairo's xlib
- * backend can work fine with 16-bit visuals in the same way it works
- * with BGR visuals without any BGR formats in
- * #cairo_format_t).
- *
- * Additionally, the support for the RGB16_565 format was never
- * completely implemented. So while this format value is currently
- * deprecated, it may eventually acquire complete support in the future.
- */
-#define CAIRO_FORMAT_RGB16_565 4
-
#ifndef _CAIROINT_H_
/* Obsolete functions. These definitions exist to coerce the compiler
diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c
index 441947f..ee73615 100644
--- a/src/cairo-image-surface.c
+++ b/src/cairo-image-surface.c
@@ -90,6 +90,8 @@ _cairo_format_bpp (cairo_format_t format)
return 1;
case CAIRO_FORMAT_A8:
return 8;
+ case CAIRO_FORMAT_RGB16_565:
+ return 16;
case CAIRO_FORMAT_RGB24:
case CAIRO_FORMAT_ARGB32:
return 32;
@@ -254,6 +256,9 @@ _init_pixman_format (pixman_format_t *pixman_format, cairo_format_t format)
case CAIRO_FORMAT_A8:
ret = pixman_format_init (pixman_format, PIXMAN_FORMAT_NAME_A8);
break;
+ case CAIRO_FORMAT_RGB16_565:
+ ret = pixman_format_init (pixman_format, PIXMAN_FORMAT_NAME_RGB16_565);
+ break;
case CAIRO_FORMAT_RGB24:
ret = pixman_format_init (pixman_format, PIXMAN_FORMAT_NAME_RGB24);
break;
diff --git a/src/cairo-png.c b/src/cairo-png.c
index 76e901f..46adcc2 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -115,6 +115,7 @@ write_png (cairo_surface_t *surface,
png_info *info;
png_time pt;
png_byte **rows;
+ png_bytep scanline = NULL;
png_color_16 white;
int png_color_type;
int depth;
@@ -134,6 +135,15 @@ write_png (cairo_surface_t *surface,
goto BAIL1;
}
+ if (image->format == CAIRO_FORMAT_RGB16_565) {
+ // A single scanline for upconverting 565 to 888
+ scanline = (png_bytep)malloc(image->width * 4);
+ if (scanline == NULL) {
+ status = CAIRO_STATUS_NO_MEMORY;
+ goto BAIL1;
+ }
+ }
+
for (i = 0; i < image->height; i++)
rows[i] = (png_byte *) image->data + i * image->stride;
@@ -163,6 +173,7 @@ write_png (cairo_surface_t *surface,
png_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
break;
case CAIRO_FORMAT_RGB24:
+ case CAIRO_FORMAT_RGB16_565:
depth = 8;
png_color_type = PNG_COLOR_TYPE_RGB;
break;
@@ -208,12 +219,33 @@ write_png (cairo_surface_t *surface,
if (image->format == CAIRO_FORMAT_RGB24)
png_set_filler (png, 0, PNG_FILLER_AFTER);
- png_write_image (png, rows);
+ if (image->format == CAIRO_FORMAT_RGB16_565) {
+ // upconvert the pixel values from 565 to 888
+ // we don't do this in a transform_fn because libpng does a
+ // memcopy of the data using the 888 width, which could
+ // potentialy cause an issue as it runs of the end of
+ // the 565 image.
+ int x, y;
+ for (y = 0; y < image->height; y++) {
+ short *src = (short*)(rows[y]);
+ png_bytep dest = scanline;
+ for(x=0;x<image->width;x++) {
+ *(dest++) = ((src[x]) & 0x0F800) >> 8;
+ *(dest++) = ((src[x]) & 0x007E0) >> 3;
+ *(dest++) = ((src[x]) & 0x0001F) << 3;
+ }
+ png_write_row(png, scanline);
+ }
+ } else {
+ png_write_image (png, rows);
+ }
png_write_end (png, info);
BAIL3:
png_destroy_write_struct (&png, &info);
BAIL2:
+ if(scanline)
+ free (scanline);
free (rows);
BAIL1:
_cairo_surface_release_source_image (surface, image, image_extra);
diff --git a/src/cairo.h b/src/cairo.h
index a80efde..7f3c639 100644
--- a/src/cairo.h
+++ b/src/cairo.h
@@ -1568,11 +1568,8 @@ typedef enum _cairo_format {
CAIRO_FORMAT_ARGB32,
CAIRO_FORMAT_RGB24,
CAIRO_FORMAT_A8,
- CAIRO_FORMAT_A1
- /* The value of 4 is reserved by a deprecated enum value.
- * The next format added must have an explicit value of 5.
- CAIRO_FORMAT_RGB16_565 = 4,
- */
+ CAIRO_FORMAT_A1,
+ CAIRO_FORMAT_RGB16_565,
} cairo_format_t;
cairo_public cairo_surface_t *
diff --git a/src/cairoint.h b/src/cairoint.h
index 7b0d57e..1573f1a 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -2002,7 +2002,7 @@ _cairo_surface_has_device_transform (cairo_surface_t *surface);
* to support it (at least cairo_surface_write_to_png and a few spots
* in cairo-xlib-surface.c--again see -Wswitch-enum).
*/
-#define CAIRO_FORMAT_VALID(format) ((format) <= CAIRO_FORMAT_A1)
+#define CAIRO_FORMAT_VALID(format) ((format) <= CAIRO_FORMAT_RGB16_565)
#define CAIRO_CONTENT_VALID(content) ((content) && \
(((content) & ~(CAIRO_CONTENT_COLOR | \
["mytest.c" (text/x-csrc)]
#include <stdio.h>
#include <stdlib.h>
#include <cairo.h>
#include <math.h>
#define WIDTH 175
#define HEIGHT 175
unsigned char *image;
int
main (void)
{
cairo_surface_t *surface;
cairo_t *cr;
int format, stride;
int x,y;
format = CAIRO_FORMAT_RGB16_565; stride = 2 * WIDTH;
format = CAIRO_FORMAT_ARGB32; stride = 4*WIDTH;
format = CAIRO_FORMAT_RGB24; stride = 4*WIDTH;
image = (unsigned char *)malloc(stride*HEIGHT);
switch (format) {
case CAIRO_FORMAT_RGB16_565:
for(y=0;y<HEIGHT;y++) {
unsigned short * s = (unsigned short*)(image + y * stride);
for(x=0;x<WIDTH;x++) {
if(x < WIDTH/2 && y < HEIGHT/2)
*(s++) = 0x0F800; // red
else if(x >= WIDTH/2 && y < HEIGHT/2)
*(s++) = 0x007E0; // green
else if(x < WIDTH/2 && y >= HEIGHT/2)
*(s++) = 0x0001F; // blue
else
*(s++) = 0x0FFFF; // white
}
}
break;
case CAIRO_FORMAT_RGB24:
case CAIRO_FORMAT_ARGB32:
for(y=0;y<HEIGHT;y++) {
unsigned int * s = (unsigned int*)(image + y * stride);
for(x=0;x<WIDTH;x++) {
if(x < WIDTH/2 && y < HEIGHT/2)
*(s++) = 0xFFFF0000; // red
else if(x >= WIDTH/2 && y < HEIGHT/2)
*(s++) = 0xFF00FF00; // green
else if(x < WIDTH/2 && y >= HEIGHT/2)
*(s++) = 0xFF0000FF; // blue
else
*(s++) = 0x0FFFFFFFF; // white
}
}
break;
default:
printf("Ouch in switch\n");
exit(-1);
}
surface = cairo_image_surface_create_for_data (image,
format, WIDTH, HEIGHT, stride);
if(!surface) {
printf("no surface\n");
exit(1);
}
cr = cairo_create (surface);
printf("surface format is %d\n",cairo_image_surface_get_format(surface));
if(!cr) {
printf("no create\n");
exit(1);
}
// cairo_rectangle (cr, 0, 0, WIDTH, HEIGHT);
// cairo_set_source_rgb (cr, 0, 0, 0);
// cairo_fill (cr);
cairo_surface_write_to_png (surface, "testing565.png");
cairo_destroy (cr);
cairo_surface_destroy (surface);
return 0;
}
_______________________________________________
cairo mailing list
cairo@cairographics.org
http://cairographics.org/cgi-bin/mailman/listinfo/cairo
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic