* [RFC] Framebuffer rotation patch
@ 2009-08-28 8:03 Michal Suchanek
2009-08-29 10:24 ` Michal Suchanek
2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 24+ messages in thread
From: Michal Suchanek @ 2009-08-28 8:03 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1: Type: text/plain, Size: 569 bytes --]
Hello
Sending a preliminary framebuffer rotation patch.
You can use videotest to see 4 tiles rotated from the same bitmap data.
Known issues:
- font glyphs and terminal do not rotate properly
- no accelerated blitters for rotated modes, at least the 1bit blitter
should work
To be done
- split out signed rectangle patch that changes graphics coordinates
from unsigned to signed
- make up some naming scheme and rename functions and macros accordingly
- make up some acceptable way to specify framebnuffer rotation in the
environment like gfxmode
Thanks
Michal
[-- Attachment #2: grub-fbtran.patch --]
[-- Type: text/x-diff, Size: 46950 bytes --]
diff --git a/commands/videotest.c b/commands/videotest.c
index 6fe4b9b..22e6e8c 100644
--- a/commands/videotest.c
+++ b/commands/videotest.c
@@ -24,6 +24,105 @@
#include <grub/font.h>
#include <grub/term.h>
#include <grub/command.h>
+#include <grub/fbtran.h>
+#include <grub/fbfill.h> /*for some reason render target type is defined there */
+#include <grub/bitmap.h>
+
+char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
+ 0x00, 0x7f, 0xfc, 0x00,
+ 0x01, 0xff, 0xff, 0x00,
+ 0x03, 0xff, 0xff, 0x80,
+
+ 0x07, 0xff, 0xff, 0xe0,
+ 0x0f, 0xff, 0xff, 0xf0,
+ 0x1f, 0xff, 0xff, 0xf0,
+ 0x3f, 0xff, 0xff, 0xf8,
+
+ 0x3f, 0xff, 0xff, 0xf8,
+ 0x7f, 0xff, 0xff, 0xfc,
+ 0x7f, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xf8,
+ 0xff, 0xff, 0xff, 0xf8,
+
+ 0xff, 0xff, 0xff, 0xf0,
+ 0xff, 0xff, 0xff, 0xe0,
+ 0xff, 0xff, 0xff, 0xc0,
+ 0xff, 0xff, 0xff, 0x80,
+
+ 0xff, 0xff, 0xff, 0x00,
+ 0xff, 0xff, 0xfc, 0x00,
+ 0xff, 0xff, 0xe0, 0x00,
+ 0xff, 0x00, 0x00, 0x00 };
+#define LEAF_SIZE 32
+
+struct grub_video_bitmap leaves[4];
+
+static void init_leaves(void)
+{
+
+ leaves[0].mode_info.width = LEAF_SIZE;
+ leaves[0].mode_info.height = LEAF_SIZE;
+ leaves[0].mode_info.transform = FB_TRAN_EAST;
+ leaves[0].mode_info.mode_type =
+ (1 << GRUB_VIDEO_MODE_TYPE_DEPTH_POS)
+ | GRUB_VIDEO_MODE_TYPE_1BIT_BITMAP;
+ leaves[0].mode_info.blit_format = GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED;
+ leaves[0].mode_info.bpp = 1;
+ leaves[0].mode_info.bytes_per_pixel = 0;
+ leaves[0].mode_info.pitch = LEAF_SIZE;
+ leaves[0].mode_info.number_of_colors = 2;
+ leaves[0].mode_info.bg_red = 128;
+ leaves[0].mode_info.fg_red = 255;
+ leaves[0].mode_info.bg_green = 128;
+ leaves[0].mode_info.fg_green = 255;
+ leaves[0].mode_info.bg_blue = 128;
+ leaves[0].mode_info.fg_blue = 255;
+ leaves[0].data = leaf_data;
+
+ grub_memcpy(&leaves[1], leaves, sizeof(leaves[0]));
+ grub_memcpy(&leaves[2], leaves, 2* sizeof(leaves[0]));
+ leaves[1].mode_info.transform = FB_TRAN_NORTH;
+ leaves[2].mode_info.transform = FB_TRAN_WEST;
+ leaves[3].mode_info.transform = FB_TRAN_SOUTH;
+ leaves[0].mode_info.fg_red = 0;
+ leaves[1].mode_info.fg_green = 0;
+ leaves[2].mode_info.fg_blue = 0;
+ leaves[3].mode_info.fg_blue = 0;
+ leaves[3].mode_info.fg_green = 0;
+ leaves[3].mode_info.fg_red = 0;
+}
+
+#define BORDER 20
+
+static void draw_leaves(void)
+{
+ grub_font_t sans;
+ grub_video_color_t color;
+
+ sans = grub_font_get ("Helvetica Bold 14");
+ color = grub_video_map_rgb (255, 255, 255);
+
+ grub_video_blit_bitmap(leaves, GRUB_VIDEO_BLIT_REPLACE, BORDER, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 1, GRUB_VIDEO_BLIT_REPLACE, BORDER+LEAF_SIZE, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 2, GRUB_VIDEO_BLIT_REPLACE, BORDER+LEAF_SIZE, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 3, GRUB_VIDEO_BLIT_REPLACE, BORDER, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_font_draw_string ("Leaves", sans, color, BORDER + LEAF_SIZE / 2, 4*LEAF_SIZE + BORDER);
+
+}
static grub_err_t
grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
@@ -34,10 +133,10 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
return grub_errno;
grub_video_color_t color;
- unsigned int x;
- unsigned int y;
- unsigned int width;
- unsigned int height;
+ int x;
+ int y;
+ int width;
+ int height;
int i;
grub_font_t sansbig;
grub_font_t sans;
@@ -48,9 +147,47 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
grub_video_color_t palette[16];
const char *str;
int texty;
+ struct grub_video_fbrender_target * tgt;
+ grub_video_fb_get_active_render_target(&tgt);
+ init_leaves();
grub_video_get_viewport (&x, &y, &width, &height);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_MIRROR;
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_FLIP|FB_TRAN_MIRROR;
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_FLIP;
+ draw_leaves();
+
+ grub_getkey ();
+
+ tgt->mode_info.transform=FB_TRAN_NORTH;
+ color = grub_video_map_rgb (0, 0, 0);
+ grub_video_fill_rect (color, 0, 0, width, height);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_EAST;
+ grub_video_set_viewport (x, y, height, width);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_SOUTH;
+ grub_video_set_viewport (x, y, width, height);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_WEST;
+ grub_video_set_viewport (x, y, height, width);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_NORTH;
+ grub_video_set_viewport (x, y, width, height);
+
+ grub_getkey ();
+
grub_video_create_render_target (&text_layer, width, height,
GRUB_VIDEO_MODE_TYPE_RGB
| GRUB_VIDEO_MODE_TYPE_ALPHA);
@@ -115,7 +252,8 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
str =
"Unicode test: happy\xE2\x98\xBA \xC2\xA3 5.00"
" \xC2\xA1\xCF\x84\xC3\xA4u! "
- " \xE2\x84\xA4\xE2\x8A\x87\xE2\x84\x9D";
+ " \xE2\x84\xA4\xE2\x8A\x87\xE2\x84\x9D "
+ " \343\201\202!" /* hiragana letter a */;
color = grub_video_map_rgb (128, 128, 255);
/* All characters in the string exist in the 'Fixed 20' (10x20) font. */
diff --git a/font/font.c b/font/font.c
index a812919..176443a 100644
--- a/font/font.c
+++ b/font/font.c
@@ -987,8 +987,10 @@ grub_font_draw_glyph (struct grub_font_glyph *glyph,
if (glyph->width == 0 || glyph->height == 0)
return GRUB_ERR_NONE;
+ /* TODO: add support for bitmaps to create_bitmap */
glyph_bitmap.mode_info.width = glyph->width;
glyph_bitmap.mode_info.height = glyph->height;
+ glyph_bitmap.mode_info.transform = 0;
glyph_bitmap.mode_info.mode_type =
(1 << GRUB_VIDEO_MODE_TYPE_DEPTH_POS)
| GRUB_VIDEO_MODE_TYPE_1BIT_BITMAP;
diff --git a/include/grub/fbblit.h b/include/grub/fbblit.h
index 664f508..ae77134 100644
--- a/include/grub/fbblit.h
+++ b/include/grub/fbblit.h
@@ -28,7 +28,7 @@ void
grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y);
+ int offset_x, int offset_y, int transform);
void
grub_video_fbblit_replace_directN (struct grub_video_fbblit_info *dst,
@@ -94,7 +94,7 @@ void
grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y);
+ int offset_x, int offset_y, int transform);
void
grub_video_fbblit_blend_BGRA8888_RGBA8888 (struct grub_video_fbblit_info *dst,
diff --git a/include/grub/fbfill.h b/include/grub/fbfill.h
index c85fa12..b60a7ba 100644
--- a/include/grub/fbfill.h
+++ b/include/grub/fbfill.h
@@ -32,10 +32,10 @@ struct grub_video_fbrender_target
struct
{
- unsigned int x;
- unsigned int y;
- unsigned int width;
- unsigned int height;
+ int x;
+ int y;
+ int width;
+ int height;
} viewport;
/* Indicates whether the data has been allocated by us and must be freed
diff --git a/include/grub/fbtran.h b/include/grub/fbtran.h
new file mode 100644
index 0000000..d72be81
--- /dev/null
+++ b/include/grub/fbtran.h
@@ -0,0 +1,235 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#ifndef GRUB_FBTRAN_HEADER
+#define GRUB_FBTRAN_HEADER 1
+
+#include <grub/video.h>
+#include <grub/video_fb.h>
+#include <grub/util/misc.h>
+
+/* NOTE: This header is private header for fb driver and should not be used
+ in other parts of the code. */
+
+/* Supported operations are simple and easy to understand.
+ * MIRROR | swap image across (around) the vertical axis
+ * FLIP - swap image across the horizontal axis - upside down
+ * SWAP / swap image across the x=y axis - swap the x and y coordinates
+ *
+ * All the operations are self-negating.
+ * || = -- = // = 0 (identity)
+ *
+ * | and - are commutative.
+ *
+ * |- = -|
+ *
+ * The relationship between \ and |,- is more peculiar:
+ *
+ * \- = |\
+ * \| = -\
+ *
+ * The typical display operations used to adjust displayed picture for use with
+ * rotated display equipment and/or mirrors are FLIP, MIRROR, and rot90,
+ * rot180, rot270.
+ *
+ * The way this rotation works is somewhat confusing. If we say "rotate left"
+ * does that mean to rotate the screen left (and rotate the picture right to
+ * compensate) or the other way around?
+ *
+ * So I will try to explain in different terms for clarity.
+ * Let's say that what you normally get is "facing the north side of the
+ * screen" or "N" for short. If you turn the screen anti-clockwise you get E,
+ * etc.
+ *
+ * TODO: check that applying mirroring first works for non-square bitmaps !
+ * If we apply the simple transforms (MIRROR, FLIP) first to get the corretct
+ * picture when facing the east side of the screen we need the -\ transform.
+ *
+ * E = -rot90 = -\
+ * S = -rot180 = -\-\ = -|\\ = -|
+ * W = -rot270 = -|-\ = |\
+ *
+ * Forward transform is from user visible cordinates to framebuffer memory coordinates.
+ */
+
+#define FB_TRAN_SWAP 1
+#define FB_TRAN_FLIP 2
+#define FB_TRAN_MIRROR 4
+#define FB_TRAN_MASK 7
+#define FB_TRAN_SIMPLE 6
+#define FB_TRAN_NORTH 0
+#define FB_TRAN_EAST (FB_TRAN_FLIP | FB_TRAN_SWAP)
+#define FB_TRAN_SOUTH (FB_TRAN_FLIP | FB_TRAN_MIRROR)
+#define FB_TRAN_WEST (FB_TRAN_MIRROR | FB_TRAN_SWAP)
+
+/* internal function used for transforms */
+static inline int fb_tran_swap_tran(int transform)
+{
+ int old = transform;
+ transform &= FB_TRAN_SWAP;
+ if (old & FB_TRAN_MIRROR)
+ transform ^= FB_TRAN_FLIP;
+ if (old & FB_TRAN_FLIP)
+ transform ^= FB_TRAN_MIRROR;
+ return transform;
+}
+
+/* Return a new bitmap for the transformation which is the result of applying
+ * the transformations present in the first bitmap, and then transformations in
+ * the second bitmap. Should work on modes that have the same width and height
+ * at the point the transfom is appended. In other words: don't use.*/
+static inline int fb_tran_append(int transform_first, int transform_second)
+{
+ return (transform_first & FB_TRAN_SWAP) ?
+ (transform_first ^ fb_tran_swap_tran(transform_second)) :
+ (transform_first ^ transform_second);
+}
+
+/* Return a bitmap for transformation that negates the specified transformation. */
+static inline int fb_tran_invert(int transform)
+{
+ return (transform & FB_TRAN_SWAP) ? fb_tran_swap_tran(transform) : transform;
+}
+
+/* Create a bitmap for transformation that has to be applied after the first
+ * transformation to obtain the second transformation. */
+static inline int fb_tran_diff(int transform_first, int transform_second)
+{
+ return fb_tran_append(fb_tran_invert(transform_first), transform_second);
+}
+
+/* transform screen dimensions */
+static inline grub_err_t fb_tran_dim_back(int * width, int * height, int transform)
+{
+ if (transform & FB_TRAN_SWAP)
+ swap_int(width, height);
+ return GRUB_ERR_NONE;
+}
+
+static inline grub_err_t fb_tran_dim(unsigned * width, unsigned * height, int transform)
+{
+ if (transform & FB_TRAN_SWAP)
+ swap_unsigned(width, height);
+ return GRUB_ERR_NONE;
+}
+
+/* internal - apply coordinate transform to a point */
+static inline grub_err_t fb_tran_point_intern(int *x, int *y, int width, int height, int transform, int user_coordinates)
+{
+ if (user_coordinates && (transform & FB_TRAN_SWAP))
+ swap_int(&width, &height);
+ if (transform & FB_TRAN_MIRROR)
+ *x = width -1 - *x;
+ if (transform & FB_TRAN_FLIP)
+ *y = height -1 - *y;
+ if (transform & FB_TRAN_SWAP)
+ swap_int(x, y);
+ return GRUB_ERR_NONE;
+}
+
+/* apply coordinate transform to a point */
+static inline grub_err_t fb_tran_point(int *x, int *y, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_point_intern(x, y, mode->width, mode->height, mode->transform, 1);
+}
+
+/* apply reverse coordinate transform to a point */
+static inline grub_err_t fb_tran_point_back(int *x, int *y, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_point_intern(x, y, mode->width, mode->height, fb_tran_invert(mode->transform), 0);
+}
+
+/* internal - apply coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect_intern(int *x, int *y, int *width, int *height, int mode_width, int mode_height, int transform, int user_coordinates)
+{
+ int x2, y2;
+ grub_fb_norm_rect(x, y, width, height);
+ x2 = *x + *width;
+ y2 = *y + *height;
+ fb_tran_point_intern(x, y, mode_width, mode_height, transform, user_coordinates);
+ fb_tran_point_intern(&x2, &y2, mode_width, mode_height, transform, user_coordinates);
+ if(*x > x2)
+ {
+ swap_int(x, &x2);
+ (*x)++;
+ x2++;
+ }
+ if(*y > y2)
+ {
+ swap_int(y, &y2);
+ (*y)++;
+ y2++;
+ }
+ *width = x2 - *x;
+ *height = y2 - *y;
+ return GRUB_ERR_NONE;
+}
+
+/* apply coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect(int *x, int *y, int *width, int *height, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_rect_intern(x, y, width, height, mode->width, mode->height, mode->transform,1);
+}
+
+/* apply reverse coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect_back(int *x, int *y, int *width, int *height, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_rect_intern(x, y, width, height, mode->width, mode->height, fb_tran_invert(mode->transform),0);
+}
+
+/* requires a 4byte buffer */
+static inline void fb_tran_format(char * str, int transform)
+{
+ if (transform & FB_TRAN_MIRROR)
+ *str++ = 'M';
+ if (transform & FB_TRAN_FLIP)
+ *str++ = 'F';
+ if (transform & FB_TRAN_SWAP)
+ *str++ = 'X';
+ *str=0;
+}
+
+/* Return bitmap of transform that is to be applied during blit.
+ * The source and target point are transformed to the framebuffer coordinates
+ * so that src_x,src_y is copied to x,y.
+ */
+#include <grub/term.h> /* grub_getkey */
+static inline grub_err_t fb_tran_blit(int *src_x, int *src_y, int *width, int *height,
+ int *x, int *y, int *transform,
+ const struct grub_video_mode_info *source_mode,
+ const struct grub_video_mode_info *target_mode)
+{
+ int sx1 = *src_x;
+ int sy1 = *src_y;
+ int sx2, sy2;
+
+ fb_tran_rect(src_x, src_y, width, height, source_mode);
+ sx2 = *src_x;
+ sy2 = *src_y;
+ fb_tran_point_back(&sx2, &sy2, source_mode);
+ *x += sx2 - sx1;
+ *y += sy2 - sy1;
+ fb_tran_point(x, y, target_mode);
+ /* TODO: define blitter transform and check that this does the right thing */
+ *transform = fb_tran_diff(source_mode->transform, target_mode->transform);
+ return GRUB_ERR_NONE;
+}
+
+
+#endif /* ! GRUB_FBTRAN_HEADER */
diff --git a/include/grub/misc.h b/include/grub/misc.h
index a63a0b4..3651046 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -212,6 +212,15 @@ grub_max (long x, long y)
return y;
}
+static inline long
+grub_min (long x, long y)
+{
+ if (x > y)
+ return y;
+ else
+ return x;
+}
+
/* Rounded-up division */
static inline unsigned int
grub_div_roundup (unsigned int x, unsigned int y)
diff --git a/include/grub/util/misc.h b/include/grub/util/misc.h
index 6a93ab0..c7ebe2f 100644
--- a/include/grub/util/misc.h
+++ b/include/grub/util/misc.h
@@ -39,6 +39,9 @@
extern char *progname;
extern int verbosity;
+static inline void swap_int(int *a, int *b) { int tmp = *a; *a = *b; *b = tmp; }
+static inline void swap_unsigned(unsigned *a, unsigned *b) { unsigned tmp = *a; *a = *b; *b = tmp; }
+
void grub_util_warn (const char *fmt, ...);
void grub_util_info (const char *fmt, ...);
void grub_util_error (const char *fmt, ...) __attribute__ ((noreturn));
diff --git a/include/grub/video.h b/include/grub/video.h
index 4145db4..c07de38 100644
--- a/include/grub/video.h
+++ b/include/grub/video.h
@@ -149,6 +149,8 @@ struct grub_video_mode_info
grub_uint8_t fg_green;
grub_uint8_t fg_blue;
grub_uint8_t fg_alpha;
+
+ int transform;
};
struct grub_video_palette_data
@@ -171,7 +173,7 @@ struct grub_video_adapter
grub_err_t (*fini) (void);
grub_err_t (*setup) (unsigned int width, unsigned int height,
- unsigned int mode_type);
+ unsigned int mode_type, int transform);
grub_err_t (*get_info) (struct grub_video_mode_info *mode_info);
@@ -184,11 +186,9 @@ struct grub_video_adapter
grub_err_t (*get_palette) (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
- grub_err_t (*set_viewport) (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+ grub_err_t (*set_viewport) (int x, int y, int width, int height);
- grub_err_t (*get_viewport) (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+ grub_err_t (*get_viewport) (int *x, int *y, int *width, int *height);
grub_video_color_t (*map_color) (grub_uint32_t color_name);
@@ -203,17 +203,17 @@ struct grub_video_adapter
grub_uint8_t *blue, grub_uint8_t *alpha);
grub_err_t (*fill_rect) (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*blit_bitmap) (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*blit_render_target) (struct grub_video_render_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*scroll) (grub_video_color_t color, int dx, int dy);
@@ -258,11 +258,11 @@ grub_err_t grub_video_set_palette (unsigned int start, unsigned int count,
grub_err_t grub_video_get_palette (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
-grub_err_t grub_video_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+grub_err_t grub_video_set_viewport (int x, int y,
+ int width, int height);
-grub_err_t grub_video_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+grub_err_t grub_video_get_viewport (int *x, int *y,
+ int *width, int *height);
grub_video_color_t grub_video_map_color (grub_uint32_t color_name);
@@ -277,19 +277,18 @@ grub_err_t grub_video_unmap_color (grub_video_color_t color,
grub_uint8_t *blue, grub_uint8_t *alpha);
grub_err_t grub_video_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t grub_video_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t grub_video_blit_render_target (struct grub_video_render_target *source,
enum grub_video_blit_operators oper,
int x, int y,
int offset_x, int offset_y,
- unsigned int width,
- unsigned int height);
+ int width, int height);
grub_err_t grub_video_scroll (grub_video_color_t color, int dx, int dy);
diff --git a/include/grub/video_fb.h b/include/grub/video_fb.h
index 850946d..f1aa295 100644
--- a/include/grub/video_fb.h
+++ b/include/grub/video_fb.h
@@ -36,6 +36,22 @@ struct grub_video_fbblit_info;
struct grub_video_fbrender_target;
+/* make width and height of a rectangle non-negative */
+static inline grub_err_t grub_fb_norm_rect(int *x, int *y, int *width, int *height)
+{
+ if(*width < 0)
+ {
+ *width = -*width;
+ *x -= *width - 1;
+ }
+ if(*height < 0)
+ {
+ *height = -*height;
+ *y -= *height - 1;
+ }
+ return GRUB_ERR_NONE;
+}
+
#define GRUB_VIDEO_FBSTD_NUMCOLORS 16
extern struct grub_video_palette_data grub_video_fbstd_colors[GRUB_VIDEO_FBSTD_NUMCOLORS];
@@ -55,11 +71,11 @@ grub_err_t
grub_video_fb_set_palette (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
grub_err_t
-grub_video_fb_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+grub_video_fb_set_viewport (int x, int y,
+ int width, int height);
grub_err_t
-grub_video_fb_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+grub_video_fb_get_viewport (int *x, int *y,
+ int *width, int *height);
grub_video_color_t
grub_video_fb_map_color (grub_uint32_t color_name);
@@ -85,19 +101,19 @@ grub_video_fb_unmap_color_int (struct grub_video_fbblit_info * source,
grub_err_t
grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper, int x, int y,
int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_scroll (grub_video_color_t color, int dx, int dy);
diff --git a/video/bitmap.c b/video/bitmap.c
index 7b135a5..88c7bfd 100644
--- a/video/bitmap.c
+++ b/video/bitmap.c
@@ -75,6 +75,7 @@ grub_video_bitmap_create (struct grub_video_bitmap **bitmap,
mode_info->width = width;
mode_info->height = height;
mode_info->blit_format = blit_format;
+ mode_info->transform = 0;
switch (blit_format)
{
diff --git a/video/fb/fbblit.c b/video/fb/fbblit.c
index 705da83..abfab8c 100644
--- a/video/fb/fbblit.c
+++ b/video/fb/fbblit.c
@@ -27,16 +27,19 @@
#include <grub/misc.h>
#include <grub/types.h>
#include <grub/video.h>
+#include <grub/fbtran.h>
/* Generic replacing blitter (slow). Works for every supported format. */
void
grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y)
+ int offset_x, int offset_y, int transform)
{
int i;
int j;
+ int dx = (transform & FB_TRAN_MIRROR) ? -1 : 1;
+ int dy = (transform & FB_TRAN_FLIP) ? -1 : 1;
grub_uint8_t src_red;
grub_uint8_t src_green;
grub_uint8_t src_blue;
@@ -56,8 +59,11 @@ grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
dst_color = grub_video_fb_map_rgba (src_red, src_green,
src_blue, src_alpha);
- set_pixel (dst, x + i, y + j, dst_color);
- }
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
+ }
}
}
@@ -409,10 +415,12 @@ void
grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y)
+ int offset_x, int offset_y, int transform)
{
int i;
int j;
+ int dx = (transform & FB_TRAN_MIRROR) ? -1 : 1;
+ int dy = (transform & FB_TRAN_FLIP) ? -1 : 1;
for (j = 0; j < height; j++)
{
@@ -460,7 +468,10 @@ grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
dst_color = grub_video_fb_map_rgba (dst_red, dst_green, dst_blue,
dst_alpha);
- set_pixel (dst, x + i, y + j, dst_color);
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
}
}
}
diff --git a/video/fb/video_fb.c b/video/fb/video_fb.c
index a35dd7a..e8e7c93 100644
--- a/video/fb/video_fb.c
+++ b/video/fb/video_fb.c
@@ -23,6 +23,7 @@
#include <grub/fbblit.h>
#include <grub/fbfill.h>
#include <grub/fbutil.h>
+#include <grub/fbtran.h>
#include <grub/bitmap.h>
static struct grub_video_fbrender_target *render_target;
@@ -117,27 +118,30 @@ grub_video_fb_set_palette (unsigned int start, unsigned int count,
}
grub_err_t
-grub_video_fb_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height)
+grub_video_fb_set_viewport (int x, int y, int width, int height)
{
+ grub_fb_norm_rect(&x, &y, &width, &height);
/* Make sure viewport is withing screen dimensions. If viewport was set
to be out of the region, mark its size as zero. */
- if (x > render_target->mode_info.width)
+ int mode_width = render_target->mode_info.width;
+ int mode_height = render_target->mode_info.height;
+ fb_tran_dim_back(&mode_width, &mode_height, render_target->mode_info.transform);
+ if (x > mode_width)
{
x = 0;
width = 0;
}
- if (y > render_target->mode_info.height)
+ if (y > mode_height)
{
y = 0;
height = 0;
}
- if (x + width > render_target->mode_info.width)
+ if (x + width > mode_width)
width = render_target->mode_info.width - x;
- if (y + height > render_target->mode_info.height)
+ if (y + height > mode_height)
height = render_target->mode_info.height - y;
render_target->viewport.x = x;
@@ -149,8 +153,7 @@ grub_video_fb_set_viewport (unsigned int x, unsigned int y,
}
grub_err_t
-grub_video_fb_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height)
+grub_video_fb_get_viewport (int *x, int *y, int *width, int *height)
{
if (x) *x = render_target->viewport.x;
if (y) *y = render_target->viewport.y;
@@ -399,14 +402,15 @@ grub_video_fb_unmap_color_int (struct grub_video_fbblit_info * source,
grub_err_t
grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info target;
+ grub_fb_norm_rect(&x, &y, &width, &height);
/* Make sure there is something to do. */
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
/* Do not allow drawing out of viewport. */
@@ -434,6 +438,9 @@ grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
target.mode_info = &render_target->mode_info;
target.data = render_target->data;
+ /* transform coordinates */
+ fb_tran_rect(&x, &y, &width, &height, &render_target->mode_info);
+
/* Try to figure out more optimized version. Note that color is already
mapped to target format so we can make assumptions based on that. */
if (target.mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_BGRA_8888)
@@ -481,15 +488,20 @@ grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
/* NOTE: This function assumes that given coordinates are within bounds of
handled data. */
+/* TODO: check transform */
static void
common_blitter (struct grub_video_fbblit_info *target,
struct grub_video_fbblit_info *source,
enum grub_video_blit_operators oper, int x, int y,
- unsigned int width, unsigned int height,
+ int width, int height,
int offset_x, int offset_y)
{
+ int transform;
+ fb_tran_blit(&offset_x, &offset_y, &width, &height, &x, &y, &transform,
+ source->mode_info, target->mode_info);
if (oper == GRUB_VIDEO_BLIT_REPLACE)
{
+ if (!transform) {
/* Try to figure out more optimized version for replace operator. */
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_RGBA_8888)
{
@@ -587,13 +599,15 @@ common_blitter (struct grub_video_fbblit_info *target,
return;
}
}
+ } /* !transform */
/* No optimized replace operator found, use default (slow) blitter. */
grub_video_fbblit_replace (target, source, x, y, width, height,
- offset_x, offset_y);
+ offset_x, offset_y, transform);
}
else
{
+ if (!transform) {
/* Try to figure out more optimized blend operator. */
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_RGBA_8888)
{
@@ -674,10 +688,11 @@ common_blitter (struct grub_video_fbblit_info *target,
return;
}
}
+ } /* !transform */
/* No optimized blend operation found, use default (slow) blitter. */
grub_video_fbblit_blend (target, source, x, y, width, height,
- offset_x, offset_y);
+ offset_x, offset_y, transform);
}
}
@@ -685,27 +700,34 @@ grub_err_t
grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper, int x, int y,
int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info source;
struct grub_video_fbblit_info target;
+ int src_x = offset_x;
+ int src_y = offset_y;
+
+ /* Normalize source rectangle and shift target insert point accordingly. */
+ grub_fb_norm_rect(&offset_x, &offset_y, &width, &height);
+ x += offset_x - src_x;
+ y += offset_y - src_y;
/* Make sure there is something to do. */
if ((width == 0) || (height == 0))
return GRUB_ERR_NONE;
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
if ((x + (int)bitmap->mode_info.width) < 0)
return GRUB_ERR_NONE;
if ((y + (int)bitmap->mode_info.height) < 0)
return GRUB_ERR_NONE;
if ((offset_x >= (int)bitmap->mode_info.width)
- || (offset_x + (int)width < 0))
+ || (offset_x + width < 0))
return GRUB_ERR_NONE;
if ((offset_y >= (int)bitmap->mode_info.height)
- || (offset_y + (int)height < 0))
+ || (offset_y + height < 0))
return GRUB_ERR_NONE;
/* If we have negative coordinates, optimize drawing to minimum. */
@@ -743,16 +765,16 @@ grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
if ((y + height) > render_target->viewport.height)
height = render_target->viewport.height - y;
- if ((offset_x + width) > bitmap->mode_info.width)
+ if ((offset_x + width) > (int)bitmap->mode_info.width)
width = bitmap->mode_info.width - offset_x;
- if ((offset_y + height) > bitmap->mode_info.height)
+ if ((offset_y + height) > (int)bitmap->mode_info.height)
height = bitmap->mode_info.height - offset_y;
/* Limit drawing to source render target dimensions. */
- if (width > bitmap->mode_info.width)
+ if (width > (int)bitmap->mode_info.width)
width = bitmap->mode_info.width;
- if (height > bitmap->mode_info.height)
+ if (height > (int)bitmap->mode_info.height)
height = bitmap->mode_info.height;
/* Add viewport offset. */
@@ -776,27 +798,37 @@ grub_err_t
grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info source_info;
struct grub_video_fbblit_info target_info;
+ int source_width = source->mode_info.width;
+ int source_height = source->mode_info.height;
+ int src_x = offset_x;
+ int src_y = offset_y;
+ fb_tran_dim_back(&source_width, &source_height, source->mode_info.transform);
+
+ /* Normalize source rectangle and shift target insert point accordingly. */
+ grub_fb_norm_rect(&offset_x, &offset_y, &width, &height);
+ x += offset_x - src_x;
+ y += offset_y - src_y;
/* Make sure there is something to do. */
if ((width == 0) || (height == 0))
return GRUB_ERR_NONE;
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
- if ((x + (int)source->mode_info.width) < 0)
+ if ((x + source_width) < 0)
return GRUB_ERR_NONE;
- if ((y + (int)source->mode_info.height) < 0)
+ if ((y + source_height) < 0)
return GRUB_ERR_NONE;
- if ((offset_x >= (int)source->mode_info.width)
- || (offset_x + (int)width < 0))
+ if ((offset_x >= source_width)
+ || (offset_x + width < 0))
return GRUB_ERR_NONE;
- if ((offset_y >= (int)source->mode_info.height)
- || (offset_y + (int)height < 0))
+ if ((offset_y >= source_height)
+ || (offset_y + height < 0))
return GRUB_ERR_NONE;
/* If we have negative coordinates, optimize drawing to minimum. */
@@ -834,17 +866,17 @@ grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
if ((y + height) > render_target->viewport.height)
height = render_target->viewport.height - y;
- if ((offset_x + width) > source->mode_info.width)
- width = source->mode_info.width - offset_x;
- if ((offset_y + height) > source->mode_info.height)
- height = source->mode_info.height - offset_y;
+ if ((offset_x + width) > source_width)
+ width = source_width - offset_x;
+ if ((offset_y + height) > source_height)
+ height = source_height - offset_y;
/* Limit drawing to source render target dimensions. */
- if (width > source->mode_info.width)
- width = source->mode_info.width;
+ if (width > source_width)
+ width = source_width;
- if (height > source->mode_info.height)
- height = source->mode_info.height;
+ if (height > source_height)
+ height = source_height;
/* Add viewport offset. */
x += render_target->viewport.x;
@@ -872,6 +904,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
int src_y;
int dst_x;
int dst_y;
+ int t_width, t_height;
/* 1. Check if we have something to do. */
if ((dx == 0) && (dy == 0))
@@ -902,9 +935,16 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
dst_y = render_target->viewport.y + dy;
}
+ t_width = width;
+ t_height = height;
+ fb_tran_rect(&src_x, &src_y, &t_width, &t_height, &render_target->mode_info);
+ t_width = width;
+ t_height = height;
+ fb_tran_rect(&dst_x, &dst_y, &t_width, &t_height, &render_target->mode_info);
+
/* 2. Check if there is need to copy data. */
- if ((grub_abs (dx) < render_target->viewport.width)
- && (grub_abs (dy) < render_target->viewport.height))
+ if (((int)grub_abs (dx) < render_target->viewport.width)
+ && ((int)grub_abs (dy) < render_target->viewport.height))
{
/* 3. Move data in render target. */
struct grub_video_fbblit_info target;
@@ -916,23 +956,23 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
target.data = render_target->data;
/* Check vertical direction of the move. */
- if (dy <= 0)
+ if (dst_y <= src_y)
/* 3a. Move data upwards. */
- for (j = 0; j < height; j++)
+ for (j = 0; j < t_height; j++)
{
dst = grub_video_fb_get_video_ptr (&target, dst_x, dst_y + j);
src = grub_video_fb_get_video_ptr (&target, src_x, src_y + j);
grub_memmove (dst, src,
- width * target.mode_info->bytes_per_pixel);
+ t_width * target.mode_info->bytes_per_pixel);
}
else
/* 3b. Move data downwards. */
- for (j = (height - 1); j >= 0; j--)
+ for (j = (t_height - 1); j >= 0; j--)
{
dst = grub_video_fb_get_video_ptr (&target, dst_x, dst_y + j);
src = grub_video_fb_get_video_ptr (&target, src_x, src_y + j);
grub_memmove (dst, src,
- width * target.mode_info->bytes_per_pixel);
+ t_width * target.mode_info->bytes_per_pixel);
}
}
@@ -945,7 +985,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
grub_video_fb_fill_rect (color, 0, 0, render_target->viewport.width, dy);
else if (dy < 0)
{
- if (render_target->viewport.height < grub_abs (dy))
+ if (render_target->viewport.height < (int)grub_abs (dy))
dy = -render_target->viewport.height;
grub_video_fb_fill_rect (color, 0, render_target->viewport.height + dy,
@@ -958,7 +998,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
dx, render_target->viewport.height);
else if (dx < 0)
{
- if (render_target->viewport.width < grub_abs (dx))
+ if (render_target->viewport.width < (int)grub_abs (dx))
dx = -render_target->viewport.width;
grub_video_fb_fill_rect (color, render_target->viewport.width + dx, 0,
@@ -976,6 +1016,7 @@ grub_video_fb_create_render_target (struct grub_video_fbrender_target **result,
{
struct grub_video_fbrender_target *target;
unsigned int size;
+ int transform = render_target->mode_info.transform;
/* Validate arguments. */
if ((! result)
@@ -1001,7 +1042,12 @@ grub_video_fb_create_render_target (struct grub_video_fbrender_target **result,
target->viewport.width = width;
target->viewport.height = height;
+ /* Set up the target so that it has the same direction as the current target */
+ /* TODO: Implement other directions, too */
+ fb_tran_dim(&width, &height, transform);
+
/* Setup render target format. */
+ target->mode_info.transform = transform;
target->mode_info.width = width;
target->mode_info.height = height;
target->mode_info.mode_type = GRUB_VIDEO_MODE_TYPE_RGB
@@ -1066,10 +1112,11 @@ grub_video_fb_create_render_target_from_pointer (struct grub_video_fbrender_targ
target->viewport.y = 0;
target->viewport.width = mode_info->width;
target->viewport.height = mode_info->height;
+ fb_tran_dim_back(&(target->viewport.width), &(target->viewport.height), mode_info->transform);
/* Clear render target with black and maximum transparency. */
for (y = 0; y < mode_info->height; y++)
- grub_memset (target->data + mode_info->pitch * y, 0,
+ grub_memset ((char *)target->data + mode_info->pitch * y, 0,
mode_info->bytes_per_pixel * mode_info->width);
/* Save result to caller. */
diff --git a/video/i386/pc/vbe.c b/video/i386/pc/vbe.c
index 3efcd56..b36d034 100644
--- a/video/i386/pc/vbe.c
+++ b/video/i386/pc/vbe.c
@@ -381,7 +381,7 @@ grub_video_vbe_fini (void)
static grub_err_t
grub_video_vbe_setup (unsigned int width, unsigned int height,
- unsigned int mode_type)
+ unsigned int mode_type, int transform)
{
grub_uint16_t *p;
struct grub_vbe_mode_info_block vbe_mode_info;
@@ -499,6 +499,8 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
framebuffer.mode_info.blit_format = grub_video_get_blit_format (&framebuffer.mode_info);
+ framebuffer.mode_info.transform = transform;
+
err = grub_video_fb_create_render_target_from_pointer (&framebuffer.render_target, &framebuffer.mode_info, framebuffer.ptr);
if (err)
diff --git a/video/video.c b/video/video.c
index 36ebfd1..14672b1 100644
--- a/video/video.c
+++ b/video/video.c
@@ -21,6 +21,7 @@
#include <grub/dl.h>
#include <grub/misc.h>
#include <grub/mm.h>
+#include <grub/fbtran.h>
/* The list of video adapters registered to system. */
static grub_video_adapter_t grub_video_adapter_list;
@@ -225,8 +226,7 @@ grub_video_get_palette (unsigned int start, unsigned int count,
/* Set viewport dimensions. */
grub_err_t
-grub_video_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height)
+grub_video_set_viewport (int x, int y, int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -236,8 +236,7 @@ grub_video_set_viewport (unsigned int x, unsigned int y,
/* Get viewport dimensions. */
grub_err_t
-grub_video_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height)
+grub_video_get_viewport (int *x, int *y, int *width, int *height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -295,7 +294,7 @@ grub_video_unmap_color (grub_video_color_t color, grub_uint8_t *red,
/* Fill rectangle using specified color. */
grub_err_t
grub_video_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -308,7 +307,7 @@ grub_err_t
grub_video_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -323,7 +322,7 @@ grub_err_t
grub_video_blit_render_target (struct grub_video_render_target *target,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -412,6 +411,10 @@ grub_video_set_mode (const char *modestring,
int height = -1;
int depth = -1;
int flags = 0;
+ int transform = 0;
+ int flip = 0;
+ int mirror = 0;
+ int swap = 0;
/* Take copy of env.var. as we don't want to modify that. */
modevar = grub_strdup (modestring);
@@ -509,6 +512,10 @@ grub_video_set_mode (const char *modestring,
current_mode = tmp;
param = tmp;
value = NULL;
+ transform = 0;
+ mirror = 0;
+ flip = 0;
+ swap = 0;
/* XXX: we assume that we're in pure text mode if
no video mode is initialized. Is it always true? */
@@ -532,7 +539,36 @@ grub_video_set_mode (const char *modestring,
}
}
- /* Parse <width>x<height>[x<depth>]*/
+ /* TODO: document direction */
+ /* Parse [Dir]<width>x<height>[x<depth>]*/
+ for (;;param++)
+ switch (*param) {
+ case 'N':
+ case 'n': transform = 0;
+ break;
+ case 'E':
+ case 'e': transform = FB_TRAN_EAST;
+ break;
+ case 'S':
+ case 's': transform = FB_TRAN_SOUTH;
+ break;
+ case 'W':
+ case 'w': transform = FB_TRAN_WEST;
+ break;
+ case 'M':
+ case 'm': mirror = 1; break;
+ case 'F':
+ case 'f': flip = 1; break;
+ case 'X':
+ case 'x': swap = 1; break;
+ default: goto end_direction_parse_loop;
+ }
+end_direction_parse_loop:
+
+ if (mirror) transform ^= FB_TRAN_MIRROR;
+ if (flip) transform ^= FB_TRAN_FLIP;
+ if (swap) transform ^= FB_TRAN_SWAP;
+
/* Find width value. */
value = param;
@@ -668,7 +704,7 @@ grub_video_set_mode (const char *modestring,
}
/* Try to initialize video mode. */
- err = p->setup (width, height, flags);
+ err = p->setup (width, height, flags, transform);
if (err != GRUB_ERR_NONE)
{
p->fini ();
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2009-08-28 8:03 [RFC] Framebuffer rotation patch Michal Suchanek
@ 2009-08-29 10:24 ` Michal Suchanek
2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 24+ messages in thread
From: Michal Suchanek @ 2009-08-29 10:24 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1: Type: text/plain, Size: 3129 bytes --]
Sending an updated patch.
fbtran should be applied after signed-rect:
Use signed rectangles in video
* include/grub/video_fb.h (grub_fb_norm_rect): new inline
* include/grub/video_fb.h: make viewport, fill and blit function
prototypes use signed rectangles
* include/grub/video.h: ditto
* include/grub/fbfill (grub_video_fbrender_target): make viewport signed
* video/fb/video_fb.c: update viewport, fill and blit functions to use
signed rectangles and grub_fb_norm_rect, update casts
* video/video.c: update viewport, fill and blit functions to
signed rectangles
* commands/videotest.c (grub_cmd_videotest): make locals signed
Add simple fb coordinate transform support
* include/grub/fbtran.h: new file
* include/grub/fbtran.h: (fb_tran_swap_tran,
fb_tran_append, fb_tran_invert, fb_tran_diff, fb_tran_rect,
fb_tran_rect_back, fb_tran_rect_intern, fb_tran_point,
fb_tran_point_back, fb_tran_point_intern, fb_tran_dim,
fb_tran_dim_backi, fb_tran_blit): new inlines
* include/grub/util/misc.h (swap_int, swap_unsigned, grub_min): new inlines
* include/grub/video.h (struct grub_video_mode_info): add transform
* include/grub/video.h (grub_video_adapter.setup): add transform
* include/grub/fbblit.h (grub_video_fbblit_blend,
grub_video_fbblit_replace): add transform
* video/fb/fbblit.c (grub_video_fbblit_blend,
grub_video_fbblit_replace): add transform support
* video/fb/video_fb.c (grub_video_fb_create_render_target): copy
transform from the active target
* video/fb/video_fb.c: add coordinate transforms from user coordinates
* video/fb/video_fb.c (common_blitter): use only generic blitter when
transform is required
* video/i386/pc/vbe.c (grub_video_vbe_setup): add transform argument,
copy it to mode_info
* video/video.c (grub_video_set_mode): parse transform from the mode
string
* video/bitmap.c (grub_video_bitmap_create): set transform to 0 in
created bitmaps
* font/font.c (grub_font_draw_glyph): set transform to 0 in created bitmaps
* term/gfxterm.c (view_width, view_height): new local variables
* term/gfxterm.c: save viewport size in view_width, view_height
- use view_width, view_height insterd of mode_info
* commands/videotest.c: add transform tests
* commands/videotest.c: leaf_data, leaves: new local variables
* commands/videotest.c: init_leaves, draw_leaves: new functions
2009/8/28 Michal Suchanek <hramrach@centrum.cz>:
> Hello
>
> Sending a preliminary framebuffer rotation patch.
>
> You can use videotest to see 4 tiles rotated from the same bitmap data.
>
> Known issues:
>
> - no "accelerated" blitters for rotated modes, at least the 1bit blitter
> should work
However, I did not see any difference between the specialized 1bit
blitter and generic blitter on qemu.
>
> To be done
>
> - make up some naming scheme and rename functions and macros accordingly
> - make up some acceptable way to specify framebnuffer rotation in the
> environment like gfxmode
>
> Thanks
>
> Michal
>
[-- Attachment #2: signed-rect.patch --]
[-- Type: text/x-diff, Size: 18745 bytes --]
diff --git a/commands/videotest.c b/commands/videotest.c
index 6fe4b9b..8434b1f 100644
--- a/commands/videotest.c
+++ b/commands/videotest.c
@@ -34,10 +34,10 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
return grub_errno;
grub_video_color_t color;
- unsigned int x;
- unsigned int y;
- unsigned int width;
- unsigned int height;
+ int x;
+ int y;
+ int width;
+ int height;
int i;
grub_font_t sansbig;
grub_font_t sans;
diff --git a/include/grub/fbfill.h b/include/grub/fbfill.h
index c85fa12..b60a7ba 100644
--- a/include/grub/fbfill.h
+++ b/include/grub/fbfill.h
@@ -32,10 +32,10 @@ struct grub_video_fbrender_target
struct
{
- unsigned int x;
- unsigned int y;
- unsigned int width;
- unsigned int height;
+ int x;
+ int y;
+ int width;
+ int height;
} viewport;
/* Indicates whether the data has been allocated by us and must be freed
diff --git a/include/grub/video.h b/include/grub/video.h
index 4145db4..c24b44b 100644
--- a/include/grub/video.h
+++ b/include/grub/video.h
@@ -184,11 +184,9 @@ struct grub_video_adapter
grub_err_t (*get_palette) (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
- grub_err_t (*set_viewport) (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+ grub_err_t (*set_viewport) (int x, int y, int width, int height);
- grub_err_t (*get_viewport) (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+ grub_err_t (*get_viewport) (int *x, int *y, int *width, int *height);
grub_video_color_t (*map_color) (grub_uint32_t color_name);
@@ -203,17 +201,17 @@ struct grub_video_adapter
grub_uint8_t *blue, grub_uint8_t *alpha);
grub_err_t (*fill_rect) (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*blit_bitmap) (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*blit_render_target) (struct grub_video_render_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t (*scroll) (grub_video_color_t color, int dx, int dy);
@@ -258,11 +256,11 @@ grub_err_t grub_video_set_palette (unsigned int start, unsigned int count,
grub_err_t grub_video_get_palette (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
-grub_err_t grub_video_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+grub_err_t grub_video_set_viewport (int x, int y,
+ int width, int height);
-grub_err_t grub_video_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+grub_err_t grub_video_get_viewport (int *x, int *y,
+ int *width, int *height);
grub_video_color_t grub_video_map_color (grub_uint32_t color_name);
@@ -277,19 +275,18 @@ grub_err_t grub_video_unmap_color (grub_video_color_t color,
grub_uint8_t *blue, grub_uint8_t *alpha);
grub_err_t grub_video_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t grub_video_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t grub_video_blit_render_target (struct grub_video_render_target *source,
enum grub_video_blit_operators oper,
int x, int y,
int offset_x, int offset_y,
- unsigned int width,
- unsigned int height);
+ int width, int height);
grub_err_t grub_video_scroll (grub_video_color_t color, int dx, int dy);
diff --git a/include/grub/video_fb.h b/include/grub/video_fb.h
index 850946d..f1aa295 100644
--- a/include/grub/video_fb.h
+++ b/include/grub/video_fb.h
@@ -36,6 +36,22 @@ struct grub_video_fbblit_info;
struct grub_video_fbrender_target;
+/* make width and height of a rectangle non-negative */
+static inline grub_err_t grub_fb_norm_rect(int *x, int *y, int *width, int *height)
+{
+ if(*width < 0)
+ {
+ *width = -*width;
+ *x -= *width - 1;
+ }
+ if(*height < 0)
+ {
+ *height = -*height;
+ *y -= *height - 1;
+ }
+ return GRUB_ERR_NONE;
+}
+
#define GRUB_VIDEO_FBSTD_NUMCOLORS 16
extern struct grub_video_palette_data grub_video_fbstd_colors[GRUB_VIDEO_FBSTD_NUMCOLORS];
@@ -55,11 +71,11 @@ grub_err_t
grub_video_fb_set_palette (unsigned int start, unsigned int count,
struct grub_video_palette_data *palette_data);
grub_err_t
-grub_video_fb_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height);
+grub_video_fb_set_viewport (int x, int y,
+ int width, int height);
grub_err_t
-grub_video_fb_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height);
+grub_video_fb_get_viewport (int *x, int *y,
+ int *width, int *height);
grub_video_color_t
grub_video_fb_map_color (grub_uint32_t color_name);
@@ -85,19 +101,19 @@ grub_video_fb_unmap_color_int (struct grub_video_fbblit_info * source,
grub_err_t
grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper, int x, int y,
int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height);
+ int width, int height);
grub_err_t
grub_video_fb_scroll (grub_video_color_t color, int dx, int dy);
diff --git a/video/fb/video_fb.c b/video/fb/video_fb.c
index 5f2917d..2fb108e 100644
--- a/video/fb/video_fb.c
+++ b/video/fb/video_fb.c
@@ -117,27 +117,27 @@ grub_video_fb_set_palette (unsigned int start, unsigned int count,
}
grub_err_t
-grub_video_fb_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height)
+grub_video_fb_set_viewport (int x, int y, int width, int height)
{
+ grub_fb_norm_rect(&x, &y, &width, &height);
/* Make sure viewport is withing screen dimensions. If viewport was set
to be out of the region, mark its size as zero. */
- if (x > render_target->mode_info.width)
+ if (x > (int)render_target->mode_info.width)
{
x = 0;
width = 0;
}
- if (y > render_target->mode_info.height)
+ if (y > (int)render_target->mode_info.height)
{
y = 0;
height = 0;
}
- if (x + width > render_target->mode_info.width)
+ if (x + width > (int)render_target->mode_info.width)
width = render_target->mode_info.width - x;
- if (y + height > render_target->mode_info.height)
+ if (y + height > (int)render_target->mode_info.height)
height = render_target->mode_info.height - y;
render_target->viewport.x = x;
@@ -149,8 +149,7 @@ grub_video_fb_set_viewport (unsigned int x, unsigned int y,
}
grub_err_t
-grub_video_fb_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height)
+grub_video_fb_get_viewport (int *x, int *y, int *width, int *height)
{
if (x) *x = render_target->viewport.x;
if (y) *y = render_target->viewport.y;
@@ -399,14 +398,15 @@ grub_video_fb_unmap_color_int (struct grub_video_fbblit_info * source,
grub_err_t
grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info target;
+ grub_fb_norm_rect(&x, &y, &width, &height);
/* Make sure there is something to do. */
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
/* Do not allow drawing out of viewport. */
@@ -485,7 +485,7 @@ static void
common_blitter (struct grub_video_fbblit_info *target,
struct grub_video_fbblit_info *source,
enum grub_video_blit_operators oper, int x, int y,
- unsigned int width, unsigned int height,
+ int width, int height,
int offset_x, int offset_y)
{
if (oper == GRUB_VIDEO_BLIT_REPLACE)
@@ -751,27 +751,34 @@ grub_err_t
grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper, int x, int y,
int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info source;
struct grub_video_fbblit_info target;
+ int src_x = offset_x;
+ int src_y = offset_y;
+
+ /* Normalize source rectangle and shift target insert point accordingly. */
+ grub_fb_norm_rect(&offset_x, &offset_y, &width, &height);
+ x += offset_x - src_x;
+ y += offset_y - src_y;
/* Make sure there is something to do. */
if ((width == 0) || (height == 0))
return GRUB_ERR_NONE;
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
if ((x + (int)bitmap->mode_info.width) < 0)
return GRUB_ERR_NONE;
if ((y + (int)bitmap->mode_info.height) < 0)
return GRUB_ERR_NONE;
if ((offset_x >= (int)bitmap->mode_info.width)
- || (offset_x + (int)width < 0))
+ || (offset_x + width < 0))
return GRUB_ERR_NONE;
if ((offset_y >= (int)bitmap->mode_info.height)
- || (offset_y + (int)height < 0))
+ || (offset_y + height < 0))
return GRUB_ERR_NONE;
/* If we have negative coordinates, optimize drawing to minimum. */
@@ -809,16 +816,16 @@ grub_video_fb_blit_bitmap (struct grub_video_bitmap *bitmap,
if ((y + height) > render_target->viewport.height)
height = render_target->viewport.height - y;
- if ((offset_x + width) > bitmap->mode_info.width)
+ if ((offset_x + width) > (int)bitmap->mode_info.width)
width = bitmap->mode_info.width - offset_x;
- if ((offset_y + height) > bitmap->mode_info.height)
+ if ((offset_y + height) > (int)bitmap->mode_info.height)
height = bitmap->mode_info.height - offset_y;
/* Limit drawing to source render target dimensions. */
- if (width > bitmap->mode_info.width)
+ if (width > (int)bitmap->mode_info.width)
width = bitmap->mode_info.width;
- if (height > bitmap->mode_info.height)
+ if (height > (int)bitmap->mode_info.height)
height = bitmap->mode_info.height;
/* Add viewport offset. */
@@ -842,27 +849,34 @@ grub_err_t
grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
struct grub_video_fbblit_info source_info;
struct grub_video_fbblit_info target_info;
+ int src_x = offset_x;
+ int src_y = offset_y;
+
+ /* Normalize source rectangle and shift target insert point accordingly. */
+ grub_fb_norm_rect(&offset_x, &offset_y, &width, &height);
+ x += offset_x - src_x;
+ y += offset_y - src_y;
/* Make sure there is something to do. */
if ((width == 0) || (height == 0))
return GRUB_ERR_NONE;
- if ((x >= (int)render_target->viewport.width) || (x + (int)width < 0))
+ if ((x >= render_target->viewport.width) || (x + width < 0))
return GRUB_ERR_NONE;
- if ((y >= (int)render_target->viewport.height) || (y + (int)height < 0))
+ if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
if ((x + (int)source->mode_info.width) < 0)
return GRUB_ERR_NONE;
if ((y + (int)source->mode_info.height) < 0)
return GRUB_ERR_NONE;
if ((offset_x >= (int)source->mode_info.width)
- || (offset_x + (int)width < 0))
+ || (offset_x + width < 0))
return GRUB_ERR_NONE;
if ((offset_y >= (int)source->mode_info.height)
- || (offset_y + (int)height < 0))
+ || (offset_y + height < 0))
return GRUB_ERR_NONE;
/* If we have negative coordinates, optimize drawing to minimum. */
@@ -900,16 +914,16 @@ grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
if ((y + height) > render_target->viewport.height)
height = render_target->viewport.height - y;
- if ((offset_x + width) > source->mode_info.width)
+ if ((offset_x + width) > (int)source->mode_info.width)
width = source->mode_info.width - offset_x;
- if ((offset_y + height) > source->mode_info.height)
+ if ((offset_y + height) > (int)source->mode_info.height)
height = source->mode_info.height - offset_y;
/* Limit drawing to source render target dimensions. */
- if (width > source->mode_info.width)
+ if (width > (int)source->mode_info.width)
width = source->mode_info.width;
- if (height > source->mode_info.height)
+ if (height > (int)source->mode_info.height)
height = source->mode_info.height;
/* Add viewport offset. */
@@ -969,8 +983,8 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
}
/* 2. Check if there is need to copy data. */
- if ((grub_abs (dx) < render_target->viewport.width)
- && (grub_abs (dy) < render_target->viewport.height))
+ if (((int)grub_abs (dx) < render_target->viewport.width)
+ && ((int)grub_abs (dy) < render_target->viewport.height))
{
/* 3. Move data in render target. */
struct grub_video_fbblit_info target;
@@ -1011,7 +1025,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
grub_video_fb_fill_rect (color, 0, 0, render_target->viewport.width, dy);
else if (dy < 0)
{
- if (render_target->viewport.height < grub_abs (dy))
+ if (render_target->viewport.height < (int)grub_abs (dy))
dy = -render_target->viewport.height;
grub_video_fb_fill_rect (color, 0, render_target->viewport.height + dy,
@@ -1024,7 +1038,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
dx, render_target->viewport.height);
else if (dx < 0)
{
- if (render_target->viewport.width < grub_abs (dx))
+ if (render_target->viewport.width < (int)grub_abs (dx))
dx = -render_target->viewport.width;
grub_video_fb_fill_rect (color, render_target->viewport.width + dx, 0,
diff --git a/video/video.c b/video/video.c
index c1d66bd..29d760f 100644
--- a/video/video.c
+++ b/video/video.c
@@ -227,8 +227,7 @@ grub_video_get_palette (unsigned int start, unsigned int count,
/* Set viewport dimensions. */
grub_err_t
-grub_video_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height)
+grub_video_set_viewport (int x, int y, int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -238,8 +237,7 @@ grub_video_set_viewport (unsigned int x, unsigned int y,
/* Get viewport dimensions. */
grub_err_t
-grub_video_get_viewport (unsigned int *x, unsigned int *y,
- unsigned int *width, unsigned int *height)
+grub_video_get_viewport (int *x, int *y, int *width, int *height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -297,7 +295,7 @@ grub_video_unmap_color (grub_video_color_t color, grub_uint8_t *red,
/* Fill rectangle using specified color. */
grub_err_t
grub_video_fill_rect (grub_video_color_t color, int x, int y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -310,7 +308,7 @@ grub_err_t
grub_video_blit_bitmap (struct grub_video_bitmap *bitmap,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
@@ -325,7 +323,7 @@ grub_err_t
grub_video_blit_render_target (struct grub_video_render_target *target,
enum grub_video_blit_operators oper,
int x, int y, int offset_x, int offset_y,
- unsigned int width, unsigned int height)
+ int width, int height)
{
if (! grub_video_adapter_active)
return grub_error (GRUB_ERR_BAD_DEVICE, "No video mode activated");
[-- Attachment #3: fbtran.patch --]
[-- Type: text/x-diff, Size: 35851 bytes --]
diff --git a/commands/videotest.c b/commands/videotest.c
index 8434b1f..b3d003a 100644
--- a/commands/videotest.c
+++ b/commands/videotest.c
@@ -24,6 +24,114 @@
#include <grub/font.h>
#include <grub/term.h>
#include <grub/command.h>
+#include <grub/fbtran.h>
+#include <grub/fbfill.h> /*for some reason render target type is defined there */
+#include <grub/bitmap.h>
+
+char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
+ 0x00, 0x7f, 0xfc, 0x00,
+ 0x01, 0xff, 0xff, 0x00,
+ 0x03, 0xff, 0xff, 0x80,
+
+ 0x07, 0xff, 0xff, 0xe0,
+ 0x0f, 0xff, 0xff, 0xf0,
+ 0x1f, 0xff, 0xff, 0xf0,
+ 0x3f, 0xff, 0xff, 0xf8,
+
+ 0x3f, 0xff, 0xff, 0xf8,
+ 0x7f, 0xff, 0xff, 0xfc,
+ 0x7f, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+ 0xff, 0xff, 0xff, 0xfe,
+
+ 0xff, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xfc,
+ 0xff, 0xff, 0xff, 0xf8,
+ 0xff, 0xff, 0xff, 0xf8,
+
+ 0xff, 0xff, 0xff, 0xf0,
+ 0xff, 0xff, 0xff, 0xe0,
+ 0xff, 0xff, 0xff, 0xc0,
+ 0xff, 0xff, 0xff, 0x80,
+
+ 0xff, 0xff, 0xff, 0x00,
+ 0xff, 0xff, 0xfc, 0x00,
+ 0xff, 0xff, 0xe0, 0x00,
+ 0xff, 0x00, 0x00, 0x00 };
+#define LEAF_SIZE 32
+
+struct grub_video_bitmap leaves[4];
+
+static void init_leaves(void)
+{
+
+ leaves[0].mode_info.width = LEAF_SIZE;
+ leaves[0].mode_info.height = LEAF_SIZE;
+ leaves[0].mode_info.transform = FB_TRAN_EAST;
+ leaves[0].mode_info.mode_type =
+ (1 << GRUB_VIDEO_MODE_TYPE_DEPTH_POS)
+ | GRUB_VIDEO_MODE_TYPE_1BIT_BITMAP;
+ leaves[0].mode_info.blit_format = GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED;
+ leaves[0].mode_info.bpp = 1;
+ leaves[0].mode_info.bytes_per_pixel = 0;
+ leaves[0].mode_info.pitch = LEAF_SIZE;
+ leaves[0].mode_info.number_of_colors = 2;
+ leaves[0].mode_info.bg_red = 128;
+ leaves[0].mode_info.fg_red = 255;
+ leaves[0].mode_info.bg_green = 128;
+ leaves[0].mode_info.bg_alpha = 128;
+ leaves[0].mode_info.fg_alpha = 255;
+ leaves[0].mode_info.fg_green = 255;
+ leaves[0].mode_info.bg_blue = 128;
+ leaves[0].mode_info.fg_blue = 255;
+ leaves[0].data = leaf_data;
+
+ grub_memcpy(&leaves[1], leaves, sizeof(leaves[0]));
+ grub_memcpy(&leaves[2], leaves, 2* sizeof(leaves[0]));
+ leaves[1].mode_info.transform = FB_TRAN_NORTH;
+ leaves[2].mode_info.transform = FB_TRAN_WEST;
+ leaves[3].mode_info.transform = FB_TRAN_SOUTH;
+ leaves[0].mode_info.fg_red = 0;
+ leaves[1].mode_info.fg_green = 0;
+ leaves[2].mode_info.fg_blue = 0;
+ leaves[3].mode_info.fg_blue = 0;
+ leaves[3].mode_info.fg_green = 0;
+ leaves[3].mode_info.fg_red = 0;
+}
+
+#define BORDER 20
+
+static void draw_leaves(void)
+{
+ grub_font_t sans;
+ grub_video_color_t text_color, fill_color;
+
+ sans = grub_font_get ("Helvetica Bold 14");
+ text_color = grub_video_map_rgb (255, 255, 255);
+ fill_color = grub_video_map_rgba (0, 192, 0, 192);
+
+ grub_video_fill_rect(fill_color, 0, 0, BORDER, BORDER);
+ grub_video_blit_bitmap(leaves, GRUB_VIDEO_BLIT_REPLACE, BORDER, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 1, GRUB_VIDEO_BLIT_REPLACE, BORDER+LEAF_SIZE, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 2, GRUB_VIDEO_BLIT_REPLACE, BORDER+LEAF_SIZE, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 3, GRUB_VIDEO_BLIT_REPLACE, BORDER, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves, GRUB_VIDEO_BLIT_BLEND, BORDER+LEAF_SIZE*2, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 1, GRUB_VIDEO_BLIT_BLEND, BORDER+LEAF_SIZE*3, BORDER, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 2, GRUB_VIDEO_BLIT_BLEND, BORDER+LEAF_SIZE*3, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_blit_bitmap(leaves + 3, GRUB_VIDEO_BLIT_BLEND, BORDER+LEAF_SIZE*2, BORDER+LEAF_SIZE, 0, 0, LEAF_SIZE, LEAF_SIZE);
+ grub_video_fill_rect(fill_color, BORDER+LEAF_SIZE*4, BORDER, LEAF_SIZE*2, LEAF_SIZE*2);
+ grub_font_draw_string ("Leaves", sans, text_color, BORDER + LEAF_SIZE / 2, 4*LEAF_SIZE + BORDER);
+
+}
static grub_err_t
grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
@@ -49,7 +157,49 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
const char *str;
int texty;
- grub_video_get_viewport (&x, &y, &width, &height);
+ if (1) /* TODO: check that we are actually using a video_fb target */
+ {
+ struct grub_video_fbrender_target * tgt;
+ grub_video_fb_get_active_render_target(&tgt);
+
+ init_leaves();
+ grub_video_get_viewport (&x, &y, &width, &height);
+
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_MIRROR;
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_FLIP|FB_TRAN_MIRROR;
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_FLIP;
+ draw_leaves();
+
+ grub_getkey ();
+
+ tgt->mode_info.transform=FB_TRAN_NORTH;
+ color = grub_video_map_rgb (0, 0, 0);
+ grub_video_fill_rect (color, 0, 0, width, height);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_EAST;
+ grub_video_set_viewport (x, y, height, width);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_SOUTH;
+ grub_video_set_viewport (x, y, width, height);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_WEST;
+ grub_video_set_viewport (x, y, height, width);
+ draw_leaves();
+
+ tgt->mode_info.transform=FB_TRAN_NORTH;
+ grub_video_set_viewport (x, y, width, height);
+
+ grub_getkey ();
+ }
grub_video_create_render_target (&text_layer, width, height,
GRUB_VIDEO_MODE_TYPE_RGB
@@ -115,7 +265,8 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
str =
"Unicode test: happy\xE2\x98\xBA \xC2\xA3 5.00"
" \xC2\xA1\xCF\x84\xC3\xA4u! "
- " \xE2\x84\xA4\xE2\x8A\x87\xE2\x84\x9D";
+ " \xE2\x84\xA4\xE2\x8A\x87\xE2\x84\x9D"
+ " \343\201\202!" /* hiragana letter a */;
color = grub_video_map_rgb (128, 128, 255);
/* All characters in the string exist in the 'Fixed 20' (10x20) font. */
diff --git a/font/font.c b/font/font.c
index a812919..176443a 100644
--- a/font/font.c
+++ b/font/font.c
@@ -987,8 +987,10 @@ grub_font_draw_glyph (struct grub_font_glyph *glyph,
if (glyph->width == 0 || glyph->height == 0)
return GRUB_ERR_NONE;
+ /* TODO: add support for bitmaps to create_bitmap */
glyph_bitmap.mode_info.width = glyph->width;
glyph_bitmap.mode_info.height = glyph->height;
+ glyph_bitmap.mode_info.transform = 0;
glyph_bitmap.mode_info.mode_type =
(1 << GRUB_VIDEO_MODE_TYPE_DEPTH_POS)
| GRUB_VIDEO_MODE_TYPE_1BIT_BITMAP;
diff --git a/include/grub/fbblit.h b/include/grub/fbblit.h
index af97dfb..3d63c9e 100644
--- a/include/grub/fbblit.h
+++ b/include/grub/fbblit.h
@@ -28,7 +28,7 @@ void
grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y);
+ int offset_x, int offset_y, int transform);
void
grub_video_fbblit_replace_directN (struct grub_video_fbblit_info *dst,
@@ -94,7 +94,7 @@ void
grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y);
+ int offset_x, int offset_y, int transform);
void
grub_video_fbblit_blend_BGRA8888_RGBA8888 (struct grub_video_fbblit_info *dst,
diff --git a/include/grub/fbtran.h b/include/grub/fbtran.h
new file mode 100644
index 0000000..407953d
--- /dev/null
+++ b/include/grub/fbtran.h
@@ -0,0 +1,220 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#ifndef GRUB_FBTRAN_HEADER
+#define GRUB_FBTRAN_HEADER 1
+
+#include <grub/video.h>
+#include <grub/video_fb.h>
+#include <grub/util/misc.h>
+
+/* NOTE: This header is private header for fb driver and should not be used
+ in other parts of the code. */
+
+/* Supported operations are simple and easy to understand.
+ * MIRROR | swap image across (around) the vertical axis
+ * FLIP - swap image across the horizontal axis - upside down
+ * SWAP / swap image across the x=y axis - swap the x and y coordinates
+ *
+ * All the operations are self-negating.
+ * || = -- = // = 0 (identity)
+ *
+ * | and - are commutative.
+ *
+ * |- = -|
+ *
+ * The relationship between \ and |,- is more peculiar:
+ *
+ * \- = |\
+ * \| = -\
+ *
+ * The typical display operations used to adjust displayed picture for use with
+ * rotated display equipment and/or mirrors are FLIP, MIRROR, and rot90,
+ * rot180, rot270.
+ *
+ * The way this rotation works is somewhat confusing. If we say "rotate left"
+ * does that mean to rotate the screen left (and rotate the picture right to
+ * compensate) or the other way around?
+ *
+ * So I will try to explain in different terms for clarity.
+ * Let's say that what you normally get is "facing the north side of the
+ * screen" or "N" for short. If you turn the screen anti-clockwise you get E,
+ * etc.
+ *
+ * If we apply the simple transforms (MIRROR, FLIP) first to get the corretct
+ * picture when facing the east side of the screen we need the -\ transform.
+ *
+ * E = -rot90 = -\
+ * S = -rot180 = -\-\ = -|\\ = -|
+ * W = -rot270 = -|-\ = |\
+ *
+ * Forward transform is from user visible cordinates to framebuffer memory coordinates.
+ */
+
+#define FB_TRAN_SWAP 1
+#define FB_TRAN_FLIP 2
+#define FB_TRAN_MIRROR 4
+#define FB_TRAN_MASK 7
+#define FB_TRAN_SIMPLE 6
+#define FB_TRAN_NORTH 0
+#define FB_TRAN_EAST (FB_TRAN_FLIP | FB_TRAN_SWAP)
+#define FB_TRAN_SOUTH (FB_TRAN_FLIP | FB_TRAN_MIRROR)
+#define FB_TRAN_WEST (FB_TRAN_MIRROR | FB_TRAN_SWAP)
+
+/* internal function used for transforms */
+static inline int fb_tran_swap_tran(int transform)
+{
+ int old = transform;
+ transform &= FB_TRAN_SWAP;
+ if (old & FB_TRAN_MIRROR)
+ transform ^= FB_TRAN_FLIP;
+ if (old & FB_TRAN_FLIP)
+ transform ^= FB_TRAN_MIRROR;
+ return transform;
+}
+
+/* Return a new bitmap for the transformation which is the result of applying
+ * the transformations present in the first bitmap, and then transformations in
+ * the second bitmap. Should work on modes that have the same width and height
+ * at the point the transfom is appended. In other words: don't use.*/
+static inline int fb_tran_append(int transform_first, int transform_second)
+{
+ return (transform_first & FB_TRAN_SWAP) ?
+ (transform_first ^ fb_tran_swap_tran(transform_second)) :
+ (transform_first ^ transform_second);
+}
+
+/* Return a bitmap for transformation that negates the specified transformation. */
+static inline int fb_tran_invert(int transform)
+{
+ return (transform & FB_TRAN_SWAP) ? fb_tran_swap_tran(transform) : transform;
+}
+
+/* Create a bitmap for transformation that has to be applied after the first
+ * transformation to obtain the second transformation. */
+static inline int fb_tran_diff(int transform_first, int transform_second)
+{
+ return fb_tran_append(fb_tran_invert(transform_first), transform_second);
+}
+
+/* transform screen dimensions */
+static inline grub_err_t fb_tran_dim_back(int * width, int * height, int transform)
+{
+ if (transform & FB_TRAN_SWAP)
+ swap_int(width, height);
+ return GRUB_ERR_NONE;
+}
+
+static inline grub_err_t fb_tran_dim(unsigned * width, unsigned * height, int transform)
+{
+ if (transform & FB_TRAN_SWAP)
+ swap_unsigned(width, height);
+ return GRUB_ERR_NONE;
+}
+
+/* internal - apply coordinate transform to a point */
+static inline grub_err_t fb_tran_point_intern(int *x, int *y, int width, int height, int transform, int user_coordinates)
+{
+ if (user_coordinates && (transform & FB_TRAN_SWAP))
+ swap_int(&width, &height);
+ if (transform & FB_TRAN_MIRROR)
+ *x = width -1 - *x;
+ if (transform & FB_TRAN_FLIP)
+ *y = height -1 - *y;
+ if (transform & FB_TRAN_SWAP)
+ swap_int(x, y);
+ return GRUB_ERR_NONE;
+}
+
+/* apply coordinate transform to a point */
+static inline grub_err_t fb_tran_point(int *x, int *y, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_point_intern(x, y, mode->width, mode->height, mode->transform, 1);
+}
+
+/* apply reverse coordinate transform to a point */
+static inline grub_err_t fb_tran_point_back(int *x, int *y, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_point_intern(x, y, mode->width, mode->height, fb_tran_invert(mode->transform), 0);
+}
+
+/* internal - apply coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect_intern(int *x, int *y, int *width, int *height, int mode_width, int mode_height, int transform, int user_coordinates)
+{
+ int x2, y2;
+ grub_fb_norm_rect(x, y, width, height);
+ x2 = *x + *width;
+ y2 = *y + *height;
+ fb_tran_point_intern(x, y, mode_width, mode_height, transform, user_coordinates);
+ fb_tran_point_intern(&x2, &y2, mode_width, mode_height, transform, user_coordinates);
+ if(*x > x2)
+ {
+ swap_int(x, &x2);
+ (*x)++;
+ x2++;
+ }
+ if(*y > y2)
+ {
+ swap_int(y, &y2);
+ (*y)++;
+ y2++;
+ }
+ *width = x2 - *x;
+ *height = y2 - *y;
+ return GRUB_ERR_NONE;
+}
+
+/* apply coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect(int *x, int *y, int *width, int *height, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_rect_intern(x, y, width, height, mode->width, mode->height, mode->transform,1);
+}
+
+/* apply reverse coordinate transform to a rectangle */
+static inline grub_err_t fb_tran_rect_back(int *x, int *y, int *width, int *height, const struct grub_video_mode_info *mode)
+{
+ return fb_tran_rect_intern(x, y, width, height, mode->width, mode->height, fb_tran_invert(mode->transform),0);
+}
+
+/* Return bitmap of transform that is to be applied during blit.
+ * The source and target point are transformed to the framebuffer coordinates
+ * so that src_x,src_y is copied to x,y.
+ */
+static inline grub_err_t fb_tran_blit(int *src_x, int *src_y, int *width, int *height,
+ int *x, int *y, int *transform,
+ const struct grub_video_mode_info *source_mode,
+ const struct grub_video_mode_info *target_mode)
+{
+ int sx1 = *src_x;
+ int sy1 = *src_y;
+ int sx2, sy2;
+
+ fb_tran_rect(src_x, src_y, width, height, source_mode);
+ sx2 = *src_x;
+ sy2 = *src_y;
+ fb_tran_point_back(&sx2, &sy2, source_mode);
+ *x += sx2 - sx1;
+ *y += sy2 - sy1;
+ fb_tran_point(x, y, target_mode);
+ *transform = fb_tran_diff(source_mode->transform, target_mode->transform);
+ return GRUB_ERR_NONE;
+}
+
+
+#endif /* ! GRUB_FBTRAN_HEADER */
diff --git a/include/grub/misc.h b/include/grub/misc.h
index a63a0b4..3651046 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -212,6 +212,15 @@ grub_max (long x, long y)
return y;
}
+static inline long
+grub_min (long x, long y)
+{
+ if (x > y)
+ return y;
+ else
+ return x;
+}
+
/* Rounded-up division */
static inline unsigned int
grub_div_roundup (unsigned int x, unsigned int y)
diff --git a/include/grub/util/misc.h b/include/grub/util/misc.h
index 6a93ab0..c7ebe2f 100644
--- a/include/grub/util/misc.h
+++ b/include/grub/util/misc.h
@@ -39,6 +39,9 @@
extern char *progname;
extern int verbosity;
+static inline void swap_int(int *a, int *b) { int tmp = *a; *a = *b; *b = tmp; }
+static inline void swap_unsigned(unsigned *a, unsigned *b) { unsigned tmp = *a; *a = *b; *b = tmp; }
+
void grub_util_warn (const char *fmt, ...);
void grub_util_info (const char *fmt, ...);
void grub_util_error (const char *fmt, ...) __attribute__ ((noreturn));
diff --git a/include/grub/video.h b/include/grub/video.h
index c24b44b..c07de38 100644
--- a/include/grub/video.h
+++ b/include/grub/video.h
@@ -149,6 +149,8 @@ struct grub_video_mode_info
grub_uint8_t fg_green;
grub_uint8_t fg_blue;
grub_uint8_t fg_alpha;
+
+ int transform;
};
struct grub_video_palette_data
@@ -171,7 +173,7 @@ struct grub_video_adapter
grub_err_t (*fini) (void);
grub_err_t (*setup) (unsigned int width, unsigned int height,
- unsigned int mode_type);
+ unsigned int mode_type, int transform);
grub_err_t (*get_info) (struct grub_video_mode_info *mode_info);
diff --git a/term/gfxterm.c b/term/gfxterm.c
index f161499..e5b35b7 100644
--- a/term/gfxterm.c
+++ b/term/gfxterm.c
@@ -105,6 +105,9 @@ static struct grub_virtual_screen virtual_screen;
static struct grub_video_mode_info mode_info;
+static int view_width;
+static int view_height;
+
static struct grub_video_render_target *text_layer;
static unsigned int bitmap_width;
@@ -259,6 +262,7 @@ grub_gfxterm_init (void)
grub_video_color_t color;
int width;
int height;
+ int view_x, view_y;
grub_err_t err;
/* Select the font to use. */
@@ -287,14 +291,15 @@ grub_gfxterm_init (void)
if (err)
return err;
+ grub_video_get_viewport(&view_x, &view_y, &view_width, &view_height);
/* Make sure screen is black. */
color = grub_video_map_rgb (0, 0, 0);
- grub_video_fill_rect (color, 0, 0, mode_info.width, mode_info.height);
+ grub_video_fill_rect (color, 0, 0, view_width, view_height);
bitmap = 0;
/* Leave borders for virtual screen. */
- width = mode_info.width - (2 * DEFAULT_BORDER_WIDTH);
- height = mode_info.height - (2 * DEFAULT_BORDER_WIDTH);
+ width = view_width - (2 * DEFAULT_BORDER_WIDTH);
+ height = view_height - (2 * DEFAULT_BORDER_WIDTH);
/* Create virtual screen. */
if (grub_virtual_screen_setup (DEFAULT_BORDER_WIDTH, DEFAULT_BORDER_WIDTH,
@@ -306,7 +311,7 @@ grub_gfxterm_init (void)
/* Mark whole screen as dirty. */
dirty_region_reset ();
- dirty_region_add (0, 0, mode_info.width, mode_info.height);
+ dirty_region_add (0, 0, view_width, view_height);
return (grub_errno = GRUB_ERR_NONE);
}
@@ -814,7 +819,7 @@ grub_gfxterm_cls (void)
/* Clear text layer. */
grub_video_set_active_render_target (text_layer);
color = virtual_screen.bg_color;
- grub_video_fill_rect (color, 0, 0, mode_info.width, mode_info.height);
+ grub_video_fill_rect (color, 0, 0, view_width, view_height);
grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY);
/* Mark virtual screen to be redrawn. */
@@ -900,7 +905,7 @@ grub_gfxterm_background_image_cmd (grub_command_t cmd __attribute__ ((unused)),
/* Mark whole screen as dirty. */
dirty_region_reset ();
- dirty_region_add (0, 0, mode_info.width, mode_info.height);
+ dirty_region_add (0, 0, view_width, view_height);
}
/* If filename was provided, try to load that. */
@@ -920,7 +925,7 @@ grub_gfxterm_background_image_cmd (grub_command_t cmd __attribute__ ((unused)),
/* Mark whole screen as dirty. */
dirty_region_reset ();
- dirty_region_add (0, 0, mode_info.width, mode_info.height);
+ dirty_region_add (0, 0, view_width, view_height);
}
}
diff --git a/video/bitmap.c b/video/bitmap.c
index 7b135a5..88c7bfd 100644
--- a/video/bitmap.c
+++ b/video/bitmap.c
@@ -75,6 +75,7 @@ grub_video_bitmap_create (struct grub_video_bitmap **bitmap,
mode_info->width = width;
mode_info->height = height;
mode_info->blit_format = blit_format;
+ mode_info->transform = 0;
switch (blit_format)
{
diff --git a/video/fb/fbblit.c b/video/fb/fbblit.c
index d9e04a5..44b6ca7 100644
--- a/video/fb/fbblit.c
+++ b/video/fb/fbblit.c
@@ -27,16 +27,19 @@
#include <grub/misc.h>
#include <grub/types.h>
#include <grub/video.h>
+#include <grub/fbtran.h>
/* Generic replacing blitter (slow). Works for every supported format. */
void
grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y)
+ int offset_x, int offset_y, int transform)
{
int i;
int j;
+ int dx = (transform & FB_TRAN_MIRROR) ? -1 : 1;
+ int dy = (transform & FB_TRAN_FLIP) ? -1 : 1;
grub_uint8_t src_red;
grub_uint8_t src_green;
grub_uint8_t src_blue;
@@ -56,8 +59,11 @@ grub_video_fbblit_replace (struct grub_video_fbblit_info *dst,
dst_color = grub_video_fb_map_rgba (src_red, src_green,
src_blue, src_alpha);
- set_pixel (dst, x + i, y + j, dst_color);
- }
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
+ }
}
}
@@ -705,10 +711,12 @@ void
grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
struct grub_video_fbblit_info *src,
int x, int y, int width, int height,
- int offset_x, int offset_y)
+ int offset_x, int offset_y, int transform)
{
int i;
int j;
+ int dx = (transform & FB_TRAN_MIRROR) ? -1 : 1;
+ int dy = (transform & FB_TRAN_FLIP) ? -1 : 1;
for (j = 0; j < height; j++)
{
@@ -735,12 +743,17 @@ grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
if (src_alpha == 255)
{
dst_color = grub_video_fb_map_rgba (src_red, src_green,
- src_blue, src_alpha);
- set_pixel (dst, x + i, y + j, dst_color);
+ src_blue, src_alpha);
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
continue;
}
- dst_color = get_pixel (dst, x + i, y + j);
+ dst_color = (transform & FB_TRAN_SWAP) ?
+ get_pixel (dst, x + j*dy, y + i*dx) :
+ get_pixel (dst, x + i*dx, y + j*dy) ;
grub_video_fb_unmap_color_int (dst, dst_color, &dst_red,
&dst_green, &dst_blue, &dst_alpha);
@@ -756,7 +769,10 @@ grub_video_fbblit_blend (struct grub_video_fbblit_info *dst,
dst_color = grub_video_fb_map_rgba (dst_red, dst_green, dst_blue,
dst_alpha);
- set_pixel (dst, x + i, y + j, dst_color);
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
}
}
}
diff --git a/video/fb/video_fb.c b/video/fb/video_fb.c
index 2fb108e..84001b8 100644
--- a/video/fb/video_fb.c
+++ b/video/fb/video_fb.c
@@ -23,6 +23,7 @@
#include <grub/fbblit.h>
#include <grub/fbfill.h>
#include <grub/fbutil.h>
+#include <grub/fbtran.h>
#include <grub/bitmap.h>
static struct grub_video_fbrender_target *render_target;
@@ -122,22 +123,25 @@ grub_video_fb_set_viewport (int x, int y, int width, int height)
grub_fb_norm_rect(&x, &y, &width, &height);
/* Make sure viewport is withing screen dimensions. If viewport was set
to be out of the region, mark its size as zero. */
- if (x > (int)render_target->mode_info.width)
+ int mode_width = render_target->mode_info.width;
+ int mode_height = render_target->mode_info.height;
+ fb_tran_dim_back(&mode_width, &mode_height, render_target->mode_info.transform);
+ if (x > mode_width)
{
x = 0;
width = 0;
}
- if (y > (int)render_target->mode_info.height)
+ if (y > mode_height)
{
y = 0;
height = 0;
}
- if (x + width > (int)render_target->mode_info.width)
+ if (x + width > mode_width)
width = render_target->mode_info.width - x;
- if (y + height > (int)render_target->mode_info.height)
+ if (y + height > mode_height)
height = render_target->mode_info.height - y;
render_target->viewport.x = x;
@@ -434,6 +438,9 @@ grub_video_fb_fill_rect (grub_video_color_t color, int x, int y,
target.mode_info = &render_target->mode_info;
target.data = render_target->data;
+ /* transform coordinates */
+ fb_tran_rect(&x, &y, &width, &height, &render_target->mode_info);
+
/* Try to figure out more optimized version. Note that color is already
mapped to target format so we can make assumptions based on that. */
if (target.mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_BGRA_8888)
@@ -488,8 +495,12 @@ common_blitter (struct grub_video_fbblit_info *target,
int width, int height,
int offset_x, int offset_y)
{
+ int transform;
+ fb_tran_blit(&offset_x, &offset_y, &width, &height, &x, &y, &transform,
+ source->mode_info, target->mode_info);
if (oper == GRUB_VIDEO_BLIT_REPLACE)
{
+ if (!transform) {
/* Try to figure out more optimized version for replace operator. */
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_RGBA_8888)
{
@@ -618,13 +629,15 @@ common_blitter (struct grub_video_fbblit_info *target,
return;
}
}
+ } /* !transform */
/* No optimized replace operator found, use default (slow) blitter. */
grub_video_fbblit_replace (target, source, x, y, width, height,
- offset_x, offset_y);
+ offset_x, offset_y, transform);
}
else
{
+ if (!transform) {
/* Try to figure out more optimized blend operator. */
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_RGBA_8888)
{
@@ -740,10 +753,11 @@ common_blitter (struct grub_video_fbblit_info *target,
}
+ } /* !transform */
/* No optimized blend operation found, use default (slow) blitter. */
grub_video_fbblit_blend (target, source, x, y, width, height,
- offset_x, offset_y);
+ offset_x, offset_y, transform);
}
}
@@ -855,6 +869,9 @@ grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
struct grub_video_fbblit_info target_info;
int src_x = offset_x;
int src_y = offset_y;
+ int source_width = source->mode_info.width;
+ int source_height = source->mode_info.height;
+ fb_tran_dim_back(&source_width, &source_height, source->mode_info.transform);
/* Normalize source rectangle and shift target insert point accordingly. */
grub_fb_norm_rect(&offset_x, &offset_y, &width, &height);
@@ -868,14 +885,14 @@ grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
return GRUB_ERR_NONE;
if ((y >= render_target->viewport.height) || (y + height < 0))
return GRUB_ERR_NONE;
- if ((x + (int)source->mode_info.width) < 0)
+ if ((x + source_width) < 0)
return GRUB_ERR_NONE;
- if ((y + (int)source->mode_info.height) < 0)
+ if ((y + source_height) < 0)
return GRUB_ERR_NONE;
- if ((offset_x >= (int)source->mode_info.width)
+ if ((offset_x >= source_width)
|| (offset_x + width < 0))
return GRUB_ERR_NONE;
- if ((offset_y >= (int)source->mode_info.height)
+ if ((offset_y >= source_height)
|| (offset_y + height < 0))
return GRUB_ERR_NONE;
@@ -914,17 +931,17 @@ grub_video_fb_blit_render_target (struct grub_video_fbrender_target *source,
if ((y + height) > render_target->viewport.height)
height = render_target->viewport.height - y;
- if ((offset_x + width) > (int)source->mode_info.width)
- width = source->mode_info.width - offset_x;
- if ((offset_y + height) > (int)source->mode_info.height)
- height = source->mode_info.height - offset_y;
+ if ((offset_x + width) > source_width)
+ width = source_width - offset_x;
+ if ((offset_y + height) > source_height)
+ height = source_height - offset_y;
/* Limit drawing to source render target dimensions. */
- if (width > (int)source->mode_info.width)
- width = source->mode_info.width;
+ if (width > source_width)
+ width = source_width;
- if (height > (int)source->mode_info.height)
- height = source->mode_info.height;
+ if (height > source_height)
+ height = source_height;
/* Add viewport offset. */
x += render_target->viewport.x;
@@ -952,6 +969,7 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
int src_y;
int dst_x;
int dst_y;
+ int t_width, t_height;
/* 1. Check if we have something to do. */
if ((dx == 0) && (dy == 0))
@@ -982,6 +1000,13 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
dst_y = render_target->viewport.y + dy;
}
+ t_width = width;
+ t_height = height;
+ fb_tran_rect(&src_x, &src_y, &t_width, &t_height, &render_target->mode_info);
+ t_width = width;
+ t_height = height;
+ fb_tran_rect(&dst_x, &dst_y, &t_width, &t_height, &render_target->mode_info);
+
/* 2. Check if there is need to copy data. */
if (((int)grub_abs (dx) < render_target->viewport.width)
&& ((int)grub_abs (dy) < render_target->viewport.height))
@@ -996,23 +1021,23 @@ grub_video_fb_scroll (grub_video_color_t color, int dx, int dy)
target.data = render_target->data;
/* Check vertical direction of the move. */
- if (dy <= 0)
+ if (dst_y <= src_y)
/* 3a. Move data upwards. */
- for (j = 0; j < height; j++)
+ for (j = 0; j < t_height; j++)
{
dst = grub_video_fb_get_video_ptr (&target, dst_x, dst_y + j);
src = grub_video_fb_get_video_ptr (&target, src_x, src_y + j);
grub_memmove (dst, src,
- width * target.mode_info->bytes_per_pixel);
+ t_width * target.mode_info->bytes_per_pixel);
}
else
/* 3b. Move data downwards. */
- for (j = (height - 1); j >= 0; j--)
+ for (j = (t_height - 1); j >= 0; j--)
{
dst = grub_video_fb_get_video_ptr (&target, dst_x, dst_y + j);
src = grub_video_fb_get_video_ptr (&target, src_x, src_y + j);
grub_memmove (dst, src,
- width * target.mode_info->bytes_per_pixel);
+ t_width * target.mode_info->bytes_per_pixel);
}
}
@@ -1056,6 +1081,7 @@ grub_video_fb_create_render_target (struct grub_video_fbrender_target **result,
{
struct grub_video_fbrender_target *target;
unsigned int size;
+ int transform = render_target->mode_info.transform;
/* Validate arguments. */
if ((! result)
@@ -1081,7 +1107,12 @@ grub_video_fb_create_render_target (struct grub_video_fbrender_target **result,
target->viewport.width = width;
target->viewport.height = height;
+ /* Set up the target so that it has the same direction as the current target */
+ /* TODO: Implement other directions, too */
+ fb_tran_dim(&width, &height, transform);
+
/* Setup render target format. */
+ target->mode_info.transform = transform;
target->mode_info.width = width;
target->mode_info.height = height;
target->mode_info.mode_type = GRUB_VIDEO_MODE_TYPE_RGB
@@ -1146,10 +1177,11 @@ grub_video_fb_create_render_target_from_pointer (struct grub_video_fbrender_targ
target->viewport.y = 0;
target->viewport.width = mode_info->width;
target->viewport.height = mode_info->height;
+ fb_tran_dim_back(&(target->viewport.width), &(target->viewport.height), mode_info->transform);
/* Clear render target with black and maximum transparency. */
for (y = 0; y < mode_info->height; y++)
- grub_memset (target->data + mode_info->pitch * y, 0,
+ grub_memset ((char *)target->data + mode_info->pitch * y, 0,
mode_info->bytes_per_pixel * mode_info->width);
/* Save result to caller. */
diff --git a/video/i386/pc/vbe.c b/video/i386/pc/vbe.c
index 3efcd56..b36d034 100644
--- a/video/i386/pc/vbe.c
+++ b/video/i386/pc/vbe.c
@@ -381,7 +381,7 @@ grub_video_vbe_fini (void)
static grub_err_t
grub_video_vbe_setup (unsigned int width, unsigned int height,
- unsigned int mode_type)
+ unsigned int mode_type, int transform)
{
grub_uint16_t *p;
struct grub_vbe_mode_info_block vbe_mode_info;
@@ -499,6 +499,8 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
framebuffer.mode_info.blit_format = grub_video_get_blit_format (&framebuffer.mode_info);
+ framebuffer.mode_info.transform = transform;
+
err = grub_video_fb_create_render_target_from_pointer (&framebuffer.render_target, &framebuffer.mode_info, framebuffer.ptr);
if (err)
diff --git a/video/video.c b/video/video.c
index 29d760f..5f7ba75 100644
--- a/video/video.c
+++ b/video/video.c
@@ -21,6 +21,7 @@
#include <grub/dl.h>
#include <grub/misc.h>
#include <grub/mm.h>
+#include <grub/fbtran.h>
/* The list of video adapters registered to system. */
static grub_video_adapter_t grub_video_adapter_list;
@@ -412,6 +413,10 @@ grub_video_set_mode (const char *modestring,
int height = -1;
int depth = -1;
int flags = 0;
+ int transform = 0;
+ int flip = 0;
+ int mirror = 0;
+ int swap = 0;
/* Take copy of env.var. as we don't want to modify that. */
modevar = grub_strdup (modestring);
@@ -509,6 +514,10 @@ grub_video_set_mode (const char *modestring,
current_mode = tmp;
param = tmp;
value = NULL;
+ transform = 0;
+ mirror = 0;
+ flip = 0;
+ swap = 0;
/* XXX: we assume that we're in pure text mode if
no video mode is initialized. Is it always true? */
@@ -532,7 +541,36 @@ grub_video_set_mode (const char *modestring,
}
}
- /* Parse <width>x<height>[x<depth>]*/
+ /* TODO: document direction */
+ /* Parse [Dir]<width>x<height>[x<depth>]*/
+ for (;;param++)
+ switch (*param) {
+ case 'N':
+ case 'n': transform = 0;
+ break;
+ case 'E':
+ case 'e': transform = FB_TRAN_EAST;
+ break;
+ case 'S':
+ case 's': transform = FB_TRAN_SOUTH;
+ break;
+ case 'W':
+ case 'w': transform = FB_TRAN_WEST;
+ break;
+ case 'M':
+ case 'm': mirror = 1; break;
+ case 'F':
+ case 'f': flip = 1; break;
+ case 'X':
+ case 'x': swap = 1; break;
+ default: goto end_direction_parse_loop;
+ }
+end_direction_parse_loop:
+
+ if (mirror) transform ^= FB_TRAN_MIRROR;
+ if (flip) transform ^= FB_TRAN_FLIP;
+ if (swap) transform ^= FB_TRAN_SWAP;
+
/* Find width value. */
value = param;
@@ -668,7 +706,7 @@ grub_video_set_mode (const char *modestring,
}
/* Try to initialize video mode. */
- err = p->setup (width, height, flags);
+ err = p->setup (width, height, flags, transform);
if (err != GRUB_ERR_NONE)
{
p->fini ();
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2009-08-28 8:03 [RFC] Framebuffer rotation patch Michal Suchanek
2009-08-29 10:24 ` Michal Suchanek
@ 2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-11 10:19 ` Michal Suchanek
2010-02-16 16:08 ` [RFC] Framebuffer rotation patch Michal Suchanek
1 sibling, 2 replies; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-11 2:46 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1: Type: text/plain, Size: 3404 bytes --]
Michal Suchanek wrote:
> Hello
>
> Sending a preliminary framebuffer rotation patch.
>
> You can use videotest to see 4 tiles rotated from the same bitmap data.
>
>
+char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
+ 0x00, 0x7f, 0xfc, 0x00,
+ 0x01, 0xff, 0xff, 0x00,
+ 0x03, 0xff, 0xff, 0x80,
This is a blob. Could it be generated automatically at build time?
+
+ sans = grub_font_get ("Helvetica Bold 14");
Please use free font rather than Helvetica
Could you split addition of videotests from the rest of the patch?
- unsigned int x;
- unsigned int y;
- unsigned int width;
- unsigned int height;
+ int x;
+ int y;
+ int width;
+ int height;
Why do you need negative values?
+/* Supported operations are simple and easy to understand.
+ * MIRROR | swap image across (around) the vertical axis
+ * FLIP - swap image across the horizontal axis - upside down
+ * SWAP / swap image across the x=y axis - swap the x and y coordinates
It's just a D_8 group. Could you add a comment to functions what they do
in group theoretical sense? It would make the code easier to follow and
compute transformations for mathematicians. Perhaps another
representation of D_8 would result in more efficient code?
+static inline void grub_swap_int(int *a, int *b) { int tmp = *a; *a =
*b; *b = tmp; }
+static inline void grub_swap_unsigned(unsigned *a, unsigned *b) {
unsigned tmp = *a; *a = *b; *b = tmp; }
+
With typeof macro this can be made type-neutral avoiding potential mistakes.
+static inline long
+grub_min (long x, long y)
+{
+ if (x > y)
+ return y;
+ else
+ return x;
+}
+
Same here
+
+ int transform;
I would prefer an enum
Skipping term/gfxterm.c since it has to be adjusted for gfxmenu-related
gfxterm.c updates.
- set_pixel (dst, x + i, y + j, dst_color);
- }
+ if (transform & FB_TRAN_SWAP)
+ set_pixel (dst, x + j*dy, y + i*dx, dst_color);
+ else
+ set_pixel (dst, x + i*dx, y + j*dy, dst_color);
+ }
Could you split it into 4 functions? I understand it's the slow fallback
function but still it should be kept as fast as possible. Perhaps you
could consider compiling same file with different #define's to obtain
function variants
grub_err_t
-grub_video_fb_set_viewport (unsigned int x, unsigned int y,
- unsigned int width, unsigned int height)
+grub_video_fb_set_viewport (int x, int y, int width, int height)
{
You don't check for x < 0 and y < 0
> Known issues:
>
> - font glyphs and terminal do not rotate properly
> - no accelerated blitters for rotated modes, at least the 1bit blitter
> should work
>
>
> To be done
>
> - split out signed rectangle patch that changes graphics coordinates
> from unsigned to signed
> - make up some naming scheme and rename functions and macros accordingly
> - make up some acceptable way to specify framebnuffer rotation in the
> environment like gfxmode
>
> Thanks
>
> Michal
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-02-11 10:19 ` Michal Suchanek
2010-02-11 10:52 ` Michal Suchanek
2010-02-11 16:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 16:08 ` [RFC] Framebuffer rotation patch Michal Suchanek
1 sibling, 2 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-11 10:19 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>> Hello
>>
>> Sending a preliminary framebuffer rotation patch.
>>
>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>
>>
> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
> + 0x00, 0x7f, 0xfc, 0x00,
> + 0x01, 0xff, 0xff, 0x00,
> + 0x03, 0xff, 0xff, 0x80,
> This is a blob. Could it be generated automatically at build time?
How less of a blob it would be if it was included as a bitmap?
This is just a shape used for the video tests.
There are some X tools for working with bitmaps/pixmaps which could
generate this from a viewable file but they are usually not available
on Windows and other non-X platforms AFAIK.
> +
> + sans = grub_font_get ("Helvetica Bold 14");
> Please use free font rather than Helvetica
I am pretty sure I did not choose the fonts, they must have been there
before I started with modifications to the tests.
> Could you split addition of videotests from the rest of the patch?
> - unsigned int x;
> - unsigned int y;
> - unsigned int width;
> - unsigned int height;
> + int x;
> + int y;
> + int width;
> + int height;
> Why do you need negative values?
I don't need the negative values everywhere but this change was to
align the typing so that casts are not required.
> +/* Supported operations are simple and easy to understand.
> + * MIRROR | swap image across (around) the vertical axis
> + * FLIP - swap image across the horizontal axis - upside down
> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
> It's just a D_8 group. Could you add a comment to functions what they do
> in group theoretical sense? It would make the code easier to follow and
Obviously it is a group, any set of transforms closed on composition is.
And according to Wikipedia it's actually called D4.
> compute transformations for mathematicians. Perhaps another
> representation of D_8 would result in more efficient code?
If you have clearer explanation or more efficient code please share.
> +static inline void grub_swap_int(int *a, int *b) { int tmp = *a; *a =
> *b; *b = tmp; }
> +static inline void grub_swap_unsigned(unsigned *a, unsigned *b) {
> unsigned tmp = *a; *a = *b; *b = tmp; }
> +
> With typeof macro this can be made type-neutral avoiding potential mistakes.
OK, I will look at typeof.
> +static inline long
> +grub_min (long x, long y)
> +{
> + if (x > y)
> + return y;
> + else
> + return x;
> +}
> +
> Same here
> +
> + int transform;
> I would prefer an enum
This is a bit field. How does it transform into an enum?
> Skipping term/gfxterm.c since it has to be adjusted for gfxmenu-related
> gfxterm.c updates.
> - set_pixel (dst, x + i, y + j, dst_color);
> - }
> + if (transform & FB_TRAN_SWAP)
> + set_pixel (dst, x + j*dy, y + i*dx, dst_color);
> + else
> + set_pixel (dst, x + i*dx, y + j*dy, dst_color);
> + }
> Could you split it into 4 functions? I understand it's the slow fallback
> function but still it should be kept as fast as possible. Perhaps you
> could consider compiling same file with different #define's to obtain
> function variants
I would rather get a patch with minimal changes first so that it's
obvious what it does.
I can look into separating this later.
> grub_err_t
> -grub_video_fb_set_viewport (unsigned int x, unsigned int y,
> - unsigned int width, unsigned int height)
> +grub_video_fb_set_viewport (int x, int y, int width, int height)
> {
> You don't check for x < 0 and y < 0
Indeed, should check for those.
>> Known issues:
>>
>> - font glyphs and terminal do not rotate properly
This should be working already.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 10:19 ` Michal Suchanek
@ 2010-02-11 10:52 ` Michal Suchanek
2010-02-11 16:11 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-11 16:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 24+ messages in thread
From: Michal Suchanek @ 2010-02-11 10:52 UTC (permalink / raw)
To: The development of GNU GRUB
On 11 February 2010 11:19, Michal Suchanek <hramrach@centrum.cz> wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> Michal Suchanek wrote:
>>> Hello
>>>
>>> Sending a preliminary framebuffer rotation patch.
>>>
>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>
>>>
>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>> + 0x00, 0x7f, 0xfc, 0x00,
>> + 0x01, 0xff, 0xff, 0x00,
>> + 0x03, 0xff, 0xff, 0x80,
>> This is a blob. Could it be generated automatically at build time?
>
> How less of a blob it would be if it was included as a bitmap?
>
> This is just a shape used for the video tests.
>
> There are some X tools for working with bitmaps/pixmaps which could
> generate this from a viewable file but they are usually not available
> on Windows and other non-X platforms AFAIK.
>
>> +
>> + sans = grub_font_get ("Helvetica Bold 14");
>> Please use free font rather than Helvetica
>
> I am pretty sure I did not choose the fonts, they must have been there
> before I started with modifications to the tests.
>
>> Could you split addition of videotests from the rest of the patch?
>> - unsigned int x;
>> - unsigned int y;
>> - unsigned int width;
>> - unsigned int height;
>> + int x;
>> + int y;
>> + int width;
>> + int height;
>> Why do you need negative values?
>
> I don't need the negative values everywhere but this change was to
> align the typing so that casts are not required.
>
>> +/* Supported operations are simple and easy to understand.
>> + * MIRROR | swap image across (around) the vertical axis
>> + * FLIP - swap image across the horizontal axis - upside down
>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>> It's just a D_8 group. Could you add a comment to functions what they do
>> in group theoretical sense? It would make the code easier to follow and
>
> Obviously it is a group, any set of transforms closed on composition is.
>
> And according to Wikipedia it's actually called D4.
>
>> compute transformations for mathematicians. Perhaps another
>> representation of D_8 would result in more efficient code?
>
> If you have clearer explanation or more efficient code please share.
>
Let me elaborate a bit more on this one.
The provided explanation should make it possible for anybody who
understands plain English to also understand the operations used and
how they interact should they know group theory in English, any other
language, or not at all.
I guess mentioning the English mathematical terms where applicable and
where I can figure them out should not that much of a problem, as well
as checking that the transform functions are sufficiently described.
Still the fact that is used (and explained) is that the operations I,
- and / generate the group and in addition you can get all members of
the group if you compose a subset of these generators in a fixed order
which is trivially true for comutative groups but not so for a
non-comutative group such as this one.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 10:19 ` Michal Suchanek
2010-02-11 10:52 ` Michal Suchanek
@ 2010-02-11 16:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-13 17:29 ` Michal Suchanek
1 sibling, 1 reply; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-11 16:08 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 4402 bytes --]
Michal Suchanek wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>
>> Michal Suchanek wrote:
>>
>>> Hello
>>>
>>> Sending a preliminary framebuffer rotation patch.
>>>
>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>
>>>
>>>
>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>> + 0x00, 0x7f, 0xfc, 0x00,
>> + 0x01, 0xff, 0xff, 0x00,
>> + 0x03, 0xff, 0xff, 0x80,
>> This is a blob. Could it be generated automatically at build time?
>>
>
> How less of a blob it would be if it was included as a bitmap?
>
>
It's not easily editable in current form.
> This is just a shape used for the video tests.
>
> There are some X tools for working with bitmaps/pixmaps which could
> generate this from a viewable file but they are usually not available
> on Windows and other non-X platforms AFAIK.
>
>
Using XBM is fine (it's easily editable in either gimp or emacs) and you
should be able to
#include "leaf1.xbm"
>> +
>> + sans = grub_font_get ("Helvetica Bold 14");
>> Please use free font rather than Helvetica
>>
>
> I am pretty sure I did not choose the fonts, they must have been there
> before I started with modifications to the tests.
>
>
Ok, it has to be fixed in mainstream then.
>> Could you split addition of videotests from the rest of the patch?
>> - unsigned int x;
>> - unsigned int y;
>> - unsigned int width;
>> - unsigned int height;
>> + int x;
>> + int y;
>> + int width;
>> + int height;
>> Why do you need negative values?
>>
>
> I don't need the negative values everywhere but this change was to
> align the typing so that casts are not required.
>
>
Perhaps few casts together with appropiate checks when casting is better
than potentially exposing the whole code to the values it's not intended
for.
>> +/* Supported operations are simple and easy to understand.
>> + * MIRROR | swap image across (around) the vertical axis
>> + * FLIP - swap image across the horizontal axis - upside down
>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>> It's just a D_8 group. Could you add a comment to functions what they do
>> in group theoretical sense? It would make the code easier to follow and
>>
>
> Obviously it is a group,
> any set of transforms closed on composition is.
>
>
Not true. Consider an example of empty set: it has no identity element.
Less trivial counter-example: set of transformations: (x,y)->(kx,ky),
0<k<1. It's non-empty but has no identity element. If you replace < 1
with <=1 you will have an identity element but no other element is
invertible.
> And according to Wikipedia it's actually called D4.
>
>
There are 2 conventions for naming dihedral groups:
1) Geometrical: since dihedral group is a symetry group of a regular
n-gone it's called D_n
2) Algebraical: since dihedral group has 2n elements it's called D_{2n}
Sometimes these conventions bear to confusion.
>> compute transformations for mathematicians. Perhaps another
>> representation of D_8 would result in more efficient code?
>>
>
> If you have clearer explanation or more efficient code please share.
>
>
I was thinking of using 2 generators instead of 3:
1) standard generators: s=rot90 or rot270, t=any reflection. Then all
elements are s^k or s^k*t. It makes computation of composition easier.
Rules on generators are:
s^n=t^2=1
tst=s^{-1}
2) or using 2 reflections. E.g. u=| and v=/. You already figured how to
generate with u=|, v=/, w=-
But w=uvuv
Then rules of computation are:
u^2=v^2=(uv)^4=1
>> +static inline long
>> +grub_min (long x, long y)
>> +{
>> + if (x > y)
>> + return y;
>> + else
>> + return x;
>> +}
>> +
>> Same here
>> +
>> + int transform;
>> I would prefer an enum
>>
>
> This is a bit field. How does it transform into an enum?
>
enum my_bitfield
{
MY_BITFIELD_NONE=0,
MY_BITFIELD_BIT0 = 1 << 0,
MY_BITFIELD_BIT1 = 1 << 1,
MY_BITFIELD_BIT2 = 1 << 2
};
> I would rather get a patch with minimal changes first so that it's
> obvious what it does.
>
> I can look into separating this later.
>
>
ok
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 10:52 ` Michal Suchanek
@ 2010-02-11 16:11 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-11 16:11 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 3368 bytes --]
Michal Suchanek wrote:
> On 11 February 2010 11:19, Michal Suchanek <hramrach@centrum.cz> wrote:
>
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>
>>> Michal Suchanek wrote:
>>>
>>>> Hello
>>>>
>>>> Sending a preliminary framebuffer rotation patch.
>>>>
>>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>>
>>>>
>>>>
>>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>>> + 0x00, 0x7f, 0xfc, 0x00,
>>> + 0x01, 0xff, 0xff, 0x00,
>>> + 0x03, 0xff, 0xff, 0x80,
>>> This is a blob. Could it be generated automatically at build time?
>>>
>> How less of a blob it would be if it was included as a bitmap?
>>
>> This is just a shape used for the video tests.
>>
>> There are some X tools for working with bitmaps/pixmaps which could
>> generate this from a viewable file but they are usually not available
>> on Windows and other non-X platforms AFAIK.
>>
>>
>>> +
>>> + sans = grub_font_get ("Helvetica Bold 14");
>>> Please use free font rather than Helvetica
>>>
>> I am pretty sure I did not choose the fonts, they must have been there
>> before I started with modifications to the tests.
>>
>>
>>> Could you split addition of videotests from the rest of the patch?
>>> - unsigned int x;
>>> - unsigned int y;
>>> - unsigned int width;
>>> - unsigned int height;
>>> + int x;
>>> + int y;
>>> + int width;
>>> + int height;
>>> Why do you need negative values?
>>>
>> I don't need the negative values everywhere but this change was to
>> align the typing so that casts are not required.
>>
>>
>>> +/* Supported operations are simple and easy to understand.
>>> + * MIRROR | swap image across (around) the vertical axis
>>> + * FLIP - swap image across the horizontal axis - upside down
>>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>>> It's just a D_8 group. Could you add a comment to functions what they do
>>> in group theoretical sense? It would make the code easier to follow and
>>>
>> Obviously it is a group, any set of transforms closed on composition is.
>>
>> And according to Wikipedia it's actually called D4.
>>
>>
>>> compute transformations for mathematicians. Perhaps another
>>> representation of D_8 would result in more efficient code?
>>>
>> If you have clearer explanation or more efficient code please share.
>>
>>
>
> Let me elaborate a bit more on this one.
>
> The provided explanation should make it possible for anybody who
> understands plain English to also understand the operations used and
> how they interact should they know group theory in English, any other
> language, or not at all.
>
Perhaps one could add an example of how it moves vertices of a square?
E.g for vertical flip:
1---2 2----1
| | --> | |
4---3 3----4
> I guess mentioning the English mathematical terms where applicable and
> where I can figure them out should not that much of a problem, as well
> as checking that the transform functions are sufficiently described.
>
>
Feel free to ask any questions.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 16:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-02-13 17:29 ` Michal Suchanek
2010-02-15 17:05 ` [RFC] Framebuffer rotation patch, or why 'unsigned' fails us Colin D Bennett
2010-02-16 12:32 ` [RFC] Framebuffer rotation patch Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-13 17:29 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>
>>> Michal Suchanek wrote:
>>>
>>>> Hello
>>>>
>>>> Sending a preliminary framebuffer rotation patch.
>>>>
>>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>>
>>>>
>>>>
>>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>>> + 0x00, 0x7f, 0xfc, 0x00,
>>> + 0x01, 0xff, 0xff, 0x00,
>>> + 0x03, 0xff, 0xff, 0x80,
>>> This is a blob. Could it be generated automatically at build time?
>>>
>>
>> How less of a blob it would be if it was included as a bitmap?
>>
>>
> It's not easily editable in current form.
>> This is just a shape used for the video tests.
>>
>> There are some X tools for working with bitmaps/pixmaps which could
>> generate this from a viewable file but they are usually not available
>> on Windows and other non-X platforms AFAIK.
>>
>>
> Using XBM is fine (it's easily editable in either gimp or emacs) and you
> should be able to
> #include "leaf1.xbm"
That would work if XBM and grub did not have opposite bit order. The
bytes are ordered the same but the bits are reversed.
>>> Could you split addition of videotests from the rest of the patch?
Yes, there should be no problem with that.
>>> - unsigned int x;
>>> - unsigned int y;
>>> - unsigned int width;
>>> - unsigned int height;
>>> + int x;
>>> + int y;
>>> + int width;
>>> + int height;
>>> Why do you need negative values?
>>>
>>
>> I don't need the negative values everywhere but this change was to
>> align the typing so that casts are not required.
>>
>>
> Perhaps few casts together with appropiate checks when casting is better
> than potentially exposing the whole code to the values it's not intended
> for.
How is it exposed to more unwanted values than now?
It uses the variables only to store the viewport dimensions and at
most adds a border around it. The unsigned integer is able to
represent values outside of the viewport as much as signed integers
are, only signed integer can represent values outside of viewport in
both directions. The unwanted values (if erroneously calculated which
does not seem to be the case here) are clipped by the viewport
clipping in video_fb.
On the other hand, there would be more than a few casts as the
variables are used multiple times and the interface now uses signed
integers.
>>> +/* Supported operations are simple and easy to understand.
>>> + * MIRROR | swap image across (around) the vertical axis
>>> + * FLIP - swap image across the horizontal axis - upside down
>>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>>> It's just a D_8 group. Could you add a comment to functions what they do
>>> in group theoretical sense? It would make the code easier to follow and
>>>
>>
>> Obviously it is a group,
>
>> any set of transforms closed on composition is.
>>
>>
> Not true. Consider an example of empty set: it has no identity element.
> Less trivial counter-example: set of transformations: (x,y)->(kx,ky),
> 0<k<1. It's non-empty but has no identity element. If you replace < 1
> with <=1 you will have an identity element but no other element is
> invertible.
It depends on your exact definition of group which somewhat varies but
it is true that most definitions at least require the set to be
non-empty.
>>
>> If you have clearer explanation or more efficient code please share.
>>
>>
> I was thinking of using 2 generators instead of 3:
> 1) standard generators: s=rot90 or rot270, t=any reflection. Then all
> elements are s^k or s^k*t. It makes computation of composition easier.
> Rules on generators are:
> s^n=t^2=1
> tst=s^{-1}
> 2) or using 2 reflections. E.g. u=| and v=/. You already figured how to
> generate with u=|, v=/, w=-
> But w=uvuv
> Then rules of computation are:
> u^2=v^2=(uv)^4=1
This might be mathematically optimal but how that maps to actual efficient code?
In my view the v (and s) operations are expensive so I want to avoid
them if possible.
This requires that both u and w be in the chosen set of generators
because otherwise use of v or s twice is required to get one from the
other. Using (u or w) and v as the two basic operations additionally
requires composing generators in non-fixed order to get all the 8
possible combinations.
The other reason for having 3 generators is clear and efficient
reperesentation of the 8 possible transformations. They are
represented as bits in the bitmap where each bit specifies if the
particular basic transform is included or not but they are always
applied in fixed order and at most once.
Compare this to your representation of s^k,s^k*t where k is 0..3.
Depending on representation the composition and inversion rules might
still be somewhat non-trivial in actual code, using two reflections
which require multiple applications of the two generators in
particular order would lead to even more "interesting" code.
>>> +static inline long
>>> +grub_min (long x, long y)
>>> +{
>>> + if (x > y)
>>> + return y;
>>> + else
>>> + return x;
>>> +}
>>> +
>>> Same here
>>> +
>>> + int transform;
>>> I would prefer an enum
>>>
>>
>> This is a bit field. How does it transform into an enum?
>>
> enum my_bitfield
> {
> MY_BITFIELD_NONE=0,
> MY_BITFIELD_BIT0 = 1 << 0,
> MY_BITFIELD_BIT1 = 1 << 1,
> MY_BITFIELD_BIT2 = 1 << 2
> };
The whole point of a bitfield is to have values like
MY_BITFIELD_1_AND_2 = MY_BITFIELD_BIT1 | MY_BITFIELD_BIT2 which enum
does not allow.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us
2010-02-13 17:29 ` Michal Suchanek
@ 2010-02-15 17:05 ` Colin D Bennett
2010-02-15 18:14 ` Michal Suchanek
2010-02-16 12:32 ` [RFC] Framebuffer rotation patch Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 24+ messages in thread
From: Colin D Bennett @ 2010-02-15 17:05 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: hramrach
On Sat, 13 Feb 2010 18:29:41 +0100
Michal Suchanek <hramrach@centrum.cz> wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> > Michal Suchanek wrote:
> >> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko
> >> <phcoder@gmail.com>:
> >>
> >>> Michal Suchanek wrote:
> >>> - unsigned int x;
> >>> - unsigned int y;
> >>> - unsigned int width;
> >>> - unsigned int height;
> >>> + int x;
> >>> + int y;
> >>> + int width;
> >>> + int height;
> >>> Why do you need negative values?
> >>>
> >>
> >> I don't need the negative values everywhere but this change was to
> >> align the typing so that casts are not required.
> >>
> >>
> > Perhaps few casts together with appropiate checks when casting is
> > better than potentially exposing the whole code to the values it's
> > not intended for.
>
> How is it exposed to more unwanted values than now?
> It uses the variables only to store the viewport dimensions and at
> most adds a border around it. The unsigned integer is able to
> represent values outside of the viewport as much as signed integers
> are, only signed integer can represent values outside of viewport in
> both directions. The unwanted values (if erroneously calculated which
> does not seem to be the case here) are clipped by the viewport
> clipping in video_fb.
>
> On the other hand, there would be more than a few casts as the
> variables are used multiple times and the interface now uses signed
> integers.
I have to chime in on this one since I've learned the hard way that
'unsigned' doesn't pull its weight in these use cases. Using unsigned
integer values in C when a lot of arithmetic is performed (as in
graphics programming) is really a nightmare. Well, maybe if you are a
grand master C programmer you would remember when you need to cast and
never make a mistake, but remember that there are us mere mortals who
would benefit by more logical and predictable code behavior. (I use
'predictable' not in the sense that expressions involving both signed
and unsigned values are have an undefined result, but rather that it's
usually not the obvious result that the average C programmer would
expect at first glance.
Certainly it would be nice if we could use the 'unsigned' qualifier to
enforce an invariant requiring only non-negative values be stored in
it (which it technically does, but it doesn't necessarily make the
result any more correct). It almost never really fixes the kind of
problems we'd like it to. We could easily try to store the result of a
subtraction that conceptually should produce a negative value into an
unsigned variable, and--OOPS!--we now have an enormous value instead of
a negative value. Not much better. More significant is code that
involves multiple operations such that the result of the entire
expression should be nonnegative, but it involves temporary negative
values in subexpressions such that the implicit signed-unsigned
conversion produces the "wrong" result (i.e., not what the programmer
intended even though the expression looks correct).
Let me quote an entry from my gfxmenu project journal, 31 May 2008:
It is worthwhile noting the potential perils of unsigned types in C
(and C++). I spent a lot of time trying to figure out why my
apparently simple code was not working. It turns out that I had
expressions that combined signed and unsigned integral types. The
unsigned types were the viewport origin/size, which GRUB defines to
be unsigned, and one of the signed types was an offset that was
added to the bitmap position during the animation. This offset had
to be signed since it could be either positive or negative,
depending on the present direction of the bitmap's movement. The
final solution was to use signed types only. I read some
discussions on signed vs. unsigned types and found this quote from
Bjarne Stroustrup, which in my experience is wise advice:
The unsigned integer types are ideal for uses that treat storage
as a bit array. Using an unsigned instead of an int to gain one
more bit to represent positive integers is almost never a good
idea. Attempts to ensure that some values are positive by
declaring variables unsigned will typically be defeated by the
implicit conversion rules.
Anyway, that's my two cents after having been bitten by arithmetic
involving both signed an unsigned values. And it's tedious to go check
the exact types of each value involved in an expression just to see
what casts you might need to produce the right result, especially when
the values involved are declared far away in some header file (i.e.,
structure members).
Regards,
Colin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us
2010-02-15 17:05 ` [RFC] Framebuffer rotation patch, or why 'unsigned' fails us Colin D Bennett
@ 2010-02-15 18:14 ` Michal Suchanek
2010-02-15 18:35 ` Colin D Bennett
2010-02-16 12:35 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-15 18:14 UTC (permalink / raw)
To: Colin D Bennett; +Cc: The development of GNU GRUB
On 15 February 2010 18:05, Colin D Bennett <colin@gibibit.com> wrote:
> On Sat, 13 Feb 2010 18:29:41 +0100
> Michal Suchanek <hramrach@centrum.cz> wrote:
>
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> > Michal Suchanek wrote:
>> >> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko
>> >> <phcoder@gmail.com>:
>> >>
>> >>> Michal Suchanek wrote:
>> >>> - unsigned int x;
>> >>> - unsigned int y;
>> >>> - unsigned int width;
>> >>> - unsigned int height;
>> >>> + int x;
>> >>> + int y;
>> >>> + int width;
>> >>> + int height;
>> >>> Why do you need negative values?
>> >>>
>> >>
>> >> I don't need the negative values everywhere but this change was to
>> >> align the typing so that casts are not required.
>> >>
>> >>
>> > Perhaps few casts together with appropiate checks when casting is
>> > better than potentially exposing the whole code to the values it's
>> > not intended for.
>>
>> How is it exposed to more unwanted values than now?
>> It uses the variables only to store the viewport dimensions and at
>> most adds a border around it. The unsigned integer is able to
>> represent values outside of the viewport as much as signed integers
>> are, only signed integer can represent values outside of viewport in
>> both directions. The unwanted values (if erroneously calculated which
>> does not seem to be the case here) are clipped by the viewport
>> clipping in video_fb.
>>
>> On the other hand, there would be more than a few casts as the
>> variables are used multiple times and the interface now uses signed
>> integers.
>
> I have to chime in on this one since I've learned the hard way that
> 'unsigned' doesn't pull its weight in these use cases. Using unsigned
> integer values in C when a lot of arithmetic is performed (as in
> graphics programming) is really a nightmare. Well, maybe if you are a
> grand master C programmer you would remember when you need to cast and
> never make a mistake, but remember that there are us mere mortals who
> would benefit by more logical and predictable code behavior. (I use
> 'predictable' not in the sense that expressions involving both signed
> and unsigned values are have an undefined result, but rather that it's
> usually not the obvious result that the average C programmer would
> expect at first glance.
>
> Certainly it would be nice if we could use the 'unsigned' qualifier to
> enforce an invariant requiring only non-negative values be stored in
> it (which it technically does, but it doesn't necessarily make the
> result any more correct). It almost never really fixes the kind of
> problems we'd like it to. We could easily try to store the result of a
> subtraction that conceptually should produce a negative value into an
> unsigned variable, and--OOPS!--we now have an enormous value instead of
> a negative value. Not much better. More significant is code that
> involves multiple operations such that the result of the entire
> expression should be nonnegative, but it involves temporary negative
> values in subexpressions such that the implicit signed-unsigned
> conversion produces the "wrong" result (i.e., not what the programmer
> intended even though the expression looks correct).
>
I started using negative integers because I need that rectangles
overflowing to the top and left are not special and different from
rectangles overflowing to the bottom and right as transforms might map
the latter to the former.
This also enhances the video interface so that drawing at negative
position is representable in the arguments. Iit is then possible to
blit a bitmap without first clipping the topleft part by just blitting
at negative position which should make some operations easier to carry
out.
What is left unsigned is the framebuffer size in mode info and it
indeed requires some casts and causes trouble. It made me change one
of the transform routines to asymmetric types because it is used
(almost) exclusively on the mode info structure. The members of
mode_info should not ever become negative, though and the unsigned
type is probably meant to make that clear. I can't say if making that
clear outweights the possible issues when using the values in
calculations.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us
2010-02-15 18:14 ` Michal Suchanek
@ 2010-02-15 18:35 ` Colin D Bennett
2010-02-16 12:35 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 24+ messages in thread
From: Colin D Bennett @ 2010-02-15 18:35 UTC (permalink / raw)
To: Michal Suchanek; +Cc: The development of GNU GRUB
On Mon, 15 Feb 2010 19:14:57 +0100
Michal Suchanek <hramrach@centrum.cz> wrote:
> What is left unsigned is the framebuffer size in mode info and it
> indeed requires some casts and causes trouble. It made me change one
> of the transform routines to asymmetric types because it is used
> (almost) exclusively on the mode info structure. The members of
> mode_info should not ever become negative, though and the unsigned
> type is probably meant to make that clear. I can't say if making that
> clear outweights the possible issues when using the values in
> calculations.
I think it is obvious that width and height values are never negative.
Since the data type (unsigned) is only visible the to programmer in the
declaration of the struct, not in uses of its members, I think it would
be just as effective to declare such values as signed types with a
comment at the declaration like "/* Nonnegative */" to make it clear,
and this would avoid signed/unsigned conversion/comparision problems.
Regards,
Colin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-13 17:29 ` Michal Suchanek
2010-02-15 17:05 ` [RFC] Framebuffer rotation patch, or why 'unsigned' fails us Colin D Bennett
@ 2010-02-16 12:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 15:52 ` Michal Suchanek
1 sibling, 1 reply; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-16 12:32 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 6826 bytes --]
Michal Suchanek wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>
>> Michal Suchanek wrote:
>>
>>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>>
>>>
>>>> Michal Suchanek wrote:
>>>>
>>>>
>>>>> Hello
>>>>>
>>>>> Sending a preliminary framebuffer rotation patch.
>>>>>
>>>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>>>
>>>>>
>>>>>
>>>>>
>>>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>>>> + 0x00, 0x7f, 0xfc, 0x00,
>>>> + 0x01, 0xff, 0xff, 0x00,
>>>> + 0x03, 0xff, 0xff, 0x80,
>>>> This is a blob. Could it be generated automatically at build time?
>>>>
>>>>
>>> How less of a blob it would be if it was included as a bitmap?
>>>
>>>
>>>
>> It's not easily editable in current form.
>>
>>> This is just a shape used for the video tests.
>>>
>>> There are some X tools for working with bitmaps/pixmaps which could
>>> generate this from a viewable file but they are usually not available
>>> on Windows and other non-X platforms AFAIK.
>>>
>>>
>>>
>> Using XBM is fine (it's easily editable in either gimp or emacs) and you
>> should be able to
>> #include "leaf1.xbm"
>>
>
> That would work if XBM and grub did not have opposite bit order. The
> bytes are ordered the same but the bits are reversed.
>
>
>>>> Could you split addition of videotests from the rest of the patch?
>>>>
>
> Yes, there should be no problem with that.
>
>
>>>> - unsigned int x;
>>>> - unsigned int y;
>>>> - unsigned int width;
>>>> - unsigned int height;
>>>> + int x;
>>>> + int y;
>>>> + int width;
>>>> + int height;
>>>> Why do you need negative values?
>>>>
>>>>
>>> I don't need the negative values everywhere but this change was to
>>> align the typing so that casts are not required.
>>>
>>>
>>>
>> Perhaps few casts together with appropiate checks when casting is better
>> than potentially exposing the whole code to the values it's not intended
>> for.
>>
>
> How is it exposed to more unwanted values than now?
> It uses the variables only to store the viewport dimensions and at
> most adds a border around it. The unsigned integer is able to
> represent values outside of the viewport as much as signed integers
> are, only signed integer can represent values outside of viewport in
> both directions. The unwanted values (if erroneously calculated which
> does not seem to be the case here) are clipped by the viewport
> clipping in video_fb.
>
> On the other hand, there would be more than a few casts as the
> variables are used multiple times and the interface now uses signed
> integers.
>
>
>>>> +/* Supported operations are simple and easy to understand.
>>>> + * MIRROR | swap image across (around) the vertical axis
>>>> + * FLIP - swap image across the horizontal axis - upside down
>>>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>>>> It's just a D_8 group. Could you add a comment to functions what they do
>>>> in group theoretical sense? It would make the code easier to follow and
>>>>
>>>>
>>> Obviously it is a group,
>>>
>>> any set of transforms closed on composition is.
>>>
>>>
>>>
>> Not true. Consider an example of empty set: it has no identity element.
>> Less trivial counter-example: set of transformations: (x,y)->(kx,ky),
>> 0<k<1. It's non-empty but has no identity element. If you replace < 1
>> with <=1 you will have an identity element but no other element is
>> invertible.
>>
>
> It depends on your exact definition of group which somewhat varies but
> it is true that most definitions at least require the set to be
> non-empty.
>
>
>>> If you have clearer explanation or more efficient code please share.
>>>
>>>
>>>
>> I was thinking of using 2 generators instead of 3:
>> 1) standard generators: s=rot90 or rot270, t=any reflection. Then all
>> elements are s^k or s^k*t. It makes computation of composition easier.
>> Rules on generators are:
>> s^n=t^2=1
>> tst=s^{-1}
>> 2) or using 2 reflections. E.g. u=| and v=/. You already figured how to
>> generate with u=|, v=/, w=-
>> But w=uvuv
>> Then rules of computation are:
>> u^2=v^2=(uv)^4=1
>>
>
> This might be mathematically optimal but how that maps to actual efficient code?
>
> In my view the v (and s) operations are expensive so I want to avoid
> them if possible.
>
I actualy though of using preprocessor magic to make 8 blitting functions.
I don't insist on any particular group representation, just want to be
sure you take it into account.
> This requires that both u and w be in the chosen set of generators
> because otherwise use of v or s twice is required to get one from the
> other. Using (u or w) and v as the two basic operations additionally
> requires composing generators in non-fixed order to get all the 8
> possible combinations.
>
> The other reason for having 3 generators is clear and efficient
> reperesentation of the 8 possible transformations. They are
> represented as bits in the bitmap where each bit specifies if the
> particular basic transform is included or not but they are always
> applied in fixed order and at most once.
>
> Compare this to your representation of s^k,s^k*t where k is 0..3.
> Depending on representation the composition and inversion rules might
> still be somewhat non-trivial in actual code, using two reflections
> which require multiple applications of the two generators in
> particular order would lead to even more "interesting" code.
>
>
You don't have to use same representation through whole code
>>>> +static inline long
>>>> +grub_min (long x, long y)
>>>> +{
>>>> + if (x > y)
>>>> + return y;
>>>> + else
>>>> + return x;
>>>> +}
>>>> +
>>>> Same here
>>>> +
>>>> + int transform;
>>>> I would prefer an enum
>>>>
>>>>
>>> This is a bit field. How does it transform into an enum?
>>>
>>>
>> enum my_bitfield
>> {
>> MY_BITFIELD_NONE=0,
>> MY_BITFIELD_BIT0 = 1 << 0,
>> MY_BITFIELD_BIT1 = 1 << 1,
>> MY_BITFIELD_BIT2 = 1 << 2
>> };
>>
>
> The whole point of a bitfield is to have values like
> MY_BITFIELD_1_AND_2 = MY_BITFIELD_BIT1 | MY_BITFIELD_BIT2 which enum
> does not allow.
>
enum allows it just fine
> Thanks
>
> Michal
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us
2010-02-15 18:14 ` Michal Suchanek
2010-02-15 18:35 ` Colin D Bennett
@ 2010-02-16 12:35 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 20:53 ` Michal Suchanek
1 sibling, 1 reply; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-16 12:35 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Colin D Bennett
[-- Attachment #1: Type: text/plain, Size: 687 bytes --]
Michal Suchanek wrote:
> This also enhances the video interface so that drawing at negative
> position is representable in the arguments. Iit is then possible to
> blit a bitmap without first clipping the topleft part by just blitting
> at negative position which should make some operations easier to carry
> out.
>
It's a good reason then. Just be sure that every code reacts sanely to
negative integers either by sanitising or correctly handling
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 12:32 ` [RFC] Framebuffer rotation patch Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-02-16 15:52 ` Michal Suchanek
2010-02-16 18:03 ` Isaac Dupree
0 siblings, 1 reply; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 15:52 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>
>>> Michal Suchanek wrote:
>> This requires that both u and w be in the chosen set of generators
>> because otherwise use of v or s twice is required to get one from the
>> other. Using (u or w) and v as the two basic operations additionally
>> requires composing generators in non-fixed order to get all the 8
>> possible combinations.
>>
>> The other reason for having 3 generators is clear and efficient
>> reperesentation of the 8 possible transformations. They are
>> represented as bits in the bitmap where each bit specifies if the
>> particular basic transform is included or not but they are always
>> applied in fixed order and at most once.
>>
>> Compare this to your representation of s^k,s^k*t where k is 0..3.
>> Depending on representation the composition and inversion rules might
>> still be somewhat non-trivial in actual code, using two reflections
>> which require multiple applications of the two generators in
>> particular order would lead to even more "interesting" code.
>>
>>
> You don't have to use same representation through whole code
I don't see any benefit in changing it and it would make the code
harder to understand.
>>>>
>>> enum my_bitfield
>>> {
>>> MY_BITFIELD_NONE=0,
>>> MY_BITFIELD_BIT0 = 1 << 0,
>>> MY_BITFIELD_BIT1 = 1 << 1,
>>> MY_BITFIELD_BIT2 = 1 << 2
>>> };
>>>
>>
>> The whole point of a bitfield is to have values like
>> MY_BITFIELD_1_AND_2 = MY_BITFIELD_BIT1 | MY_BITFIELD_BIT2 which enum
>> does not allow.
>>
> enum allows it just fine
Not here:
typedef enum t1 { BTI1 = 1,
BIT2 = 2,
BIT12 = BIT1 + BIT2} t;
int main(int argc, char** argv) { return 0 ; }
testenum.c:3: error: ‘BIT1’ undeclared here (not in a function)
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-11 10:19 ` Michal Suchanek
@ 2010-02-16 16:08 ` Michal Suchanek
2010-02-16 16:21 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 16:08 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>> Hello
>>
>> Sending a preliminary framebuffer rotation patch.
>>
>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>
>>
> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
> + 0x00, 0x7f, 0xfc, 0x00,
> + 0x01, 0xff, 0xff, 0x00,
> + 0x03, 0xff, 0xff, 0x80,
> This is a blob. Could it be generated automatically at build time?
> +
> + sans = grub_font_get ("Helvetica Bold 14");
> Please use free font rather than Helvetica
> Could you split addition of videotests from the rest of the patch?
> - unsigned int x;
> - unsigned int y;
> - unsigned int width;
> - unsigned int height;
> + int x;
> + int y;
> + int width;
> + int height;
> Why do you need negative values?
> +/* Supported operations are simple and easy to understand.
> + * MIRROR | swap image across (around) the vertical axis
> + * FLIP - swap image across the horizontal axis - upside down
> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
> It's just a D_8 group. Could you add a comment to functions what they do
> in group theoretical sense? It would make the code easier to follow and
> compute transformations for mathematicians. Perhaps another
> representation of D_8 would result in more efficient code?
> +static inline void grub_swap_int(int *a, int *b) { int tmp = *a; *a =
> *b; *b = tmp; }
> +static inline void grub_swap_unsigned(unsigned *a, unsigned *b) {
> unsigned tmp = *a; *a = *b; *b = tmp; }
> +
> With typeof macro this can be made type-neutral avoiding potential mistakes.
> +static inline long
> +grub_min (long x, long y)
> +{
> + if (x > y)
> + return y;
> + else
> + return x;
> +}
> +
I don't see how typeof would be used. As I understand the docs it can
only set types relative to something what is already defined (and in
some cases actually dereference/call it) and there is nothing defined
at the point these functions are declared to copy the type from.
Still the grub_min being long is somewhat suspicious. iirc I just
reused an existing function or copied grub_max.
If declaring it as long can cause problems then its utility as a
general purpose function is dubious.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 16:08 ` [RFC] Framebuffer rotation patch Michal Suchanek
@ 2010-02-16 16:21 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 21:05 ` Michal Suchanek
0 siblings, 1 reply; 24+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-02-16 16:21 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
Michal Suchanek wrote:
>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>> +static inline long
>> +grub_min (long x, long y)
>> +{
>> + if (x > y)
>> + return y;
>> + else
>> + return x;
>> +}
>> +
>>
>
> I don't see how typeof would be used. As I understand the docs it can
> only set types relative to something what is already defined (and in
> some cases actually dereference/call it) and there is nothing defined
> at the point these functions are declared to copy the type from.
>
#include <stdio.h>
#define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
b; b = a; a = mytemp ## __LINE__; }
int
main ()
{
int x = 1, y = 2;
swap (x,y);
printf ("%d, %d\n", x, y);
}
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 15:52 ` Michal Suchanek
@ 2010-02-16 18:03 ` Isaac Dupree
2010-02-16 18:15 ` richardvoigt
0 siblings, 1 reply; 24+ messages in thread
From: Isaac Dupree @ 2010-02-16 18:03 UTC (permalink / raw)
To: The development of GNU GRUB
On 02/16/10 10:52, Michal Suchanek wrote:
>> enum allows it just fine
>
> Not here:
>
> typedef enum t1 { BTI1 = 1,
typo, should be "BIT1". then it works. (In C. Also remember not to get
confused by the fact that it doesn't work in C++, for type-related
reasons that we don't need to get into here because GRUB is written in C.)
> BIT2 = 2,
> BIT12 = BIT1 + BIT2} t;
>
> int main(int argc, char** argv) { return 0 ; }
>
> testenum.c:3: error: ‘BIT1’ undeclared here (not in a function)
-Isaac
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 18:03 ` Isaac Dupree
@ 2010-02-16 18:15 ` richardvoigt
2010-02-16 18:47 ` [Off-topic] C++ enums Isaac Dupree
0 siblings, 1 reply; 24+ messages in thread
From: richardvoigt @ 2010-02-16 18:15 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Feb 16, 2010 at 12:03 PM, Isaac Dupree
<ml@isaac.cedarswampstudios.org> wrote:
> On 02/16/10 10:52, Michal Suchanek wrote:
>>>
>>> enum allows it just fine
>>
>> Not here:
>>
>> typedef enum t1 { BTI1 = 1,
>
> typo, should be "BIT1". then it works. (In C. Also remember not to get
> confused by the fact that it doesn't work in C++, for type-related reasons
Says who?
Comeau is perfectly happy with this code, in strict C++ mode:
enum flags
{
BIT1 = 1,
BIT2 = 2,
BIT12 = BIT1 | BIT2
};
int main(int argc, char** argv) { return 0 ; }
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Off-topic] C++ enums
2010-02-16 18:15 ` richardvoigt
@ 2010-02-16 18:47 ` Isaac Dupree
0 siblings, 0 replies; 24+ messages in thread
From: Isaac Dupree @ 2010-02-16 18:47 UTC (permalink / raw)
To: The development of GNU GRUB
On 02/16/10 13:15, richardvoigt@gmail.com wrote:
> On Tue, Feb 16, 2010 at 12:03 PM, Isaac Dupree
> <ml@isaac.cedarswampstudios.org> wrote:
>> On 02/16/10 10:52, Michal Suchanek wrote:
>>>>
>>>> enum allows it just fine
>>>
>>> Not here:
>>>
>>> typedef enum t1 { BTI1 = 1,
>>
>> typo, should be "BIT1". then it works. (In C. Also remember not to get
>> confused by the fact that it doesn't work in C++, for type-related reasons
>
> Says who?
OK, I guess I'll get into it, since you asked...
> Comeau is perfectly happy with this code, in strict C++ mode:
>
> enum flags
> {
> BIT1 = 1,
> BIT2 = 2,
> BIT12 = BIT1 | BIT2
> };
In C and C++, enums are their own type distinct from int. In C, there
exist implicit conversions both from and to all enum types and int. In
C++, there only exist implicit conversions from enum types to int.
Bitwise and arithmetic operators only operate on int, not enum types
(unless a C++ operator overload is declared); these operators appear to
work with enums because of all the implicit conversion.
Your example works even in C++ because the right-hand side of an "=" in
an enum-declaration is of type int, rather than the enum type (as you
can see from the fact that you can put "1" there as well). Now BIT1,
BIT2, and BIT12 are all (in C++) values (rvalues) of type "flags" (note
this does not apply to the left-hand sides in the enum declaration that
define their constant values, but it does apply to those right-hand sides).
Now try and combine your with this main function:
int main(int argc, char** argv) {
flags foo1 = BIT1; // fine
flags foo12 = BIT12; // fine
int bar = BIT1 | BIT2; // fine
flags baz = BIT1 | BIT2; // error
return 0 ;
}
(If you want to see it compile in C, add "typedef enum flags flags;"
before main().)
See http://www.parashift.com/c++-faq-lite/newbie.html#faq-29.19 for some
discussion by a C++ expert. And then return to GRUB2 coding where none
of this matters. The only weird thing about enums in C is that they're
not guaranteed by the standard to be isomorphic to type "int"; each enum
might correspond to a different-size, (possibly even
different-signedness), integral type, if this is possible given the
range of values in the particular "enum{...};" declaration. (This weird
thing also affects C++ but I omitted it in the above explanation of enum
versus integral types.)
-Isaac
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us
2010-02-16 12:35 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-02-16 20:53 ` Michal Suchanek
0 siblings, 0 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 20:53 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>> This also enhances the video interface so that drawing at negative
>> position is representable in the arguments. Iit is then possible to
>> blit a bitmap without first clipping the topleft part by just blitting
>> at negative position which should make some operations easier to carry
>> out.
>>
> It's a good reason then. Just be sure that every code reacts sanely to
> negative integers either by sanitising or correctly handling
Actually it was possible to blit a bitmap at negative position but it
was not possile to draw a rectangle at the same position which was
somewhat odd.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 16:21 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-02-16 21:05 ` Michal Suchanek
2010-02-16 21:14 ` richardvoigt
2010-02-16 21:53 ` Michal Suchanek
0 siblings, 2 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 21:05 UTC (permalink / raw)
To: The development of GNU GRUB
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> Michal Suchanek wrote:
>>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>>> +static inline long
>>> +grub_min (long x, long y)
>>> +{
>>> + if (x > y)
>>> + return y;
>>> + else
>>> + return x;
>>> +}
>>> +
>>>
>>
>> I don't see how typeof would be used. As I understand the docs it can
>> only set types relative to something what is already defined (and in
>> some cases actually dereference/call it) and there is nothing defined
>> at the point these functions are declared to copy the type from.
>>
> #include <stdio.h>
> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
> b; b = a; a = mytemp ## __LINE__; }
>
Unlike inlines this pollutes the local namespace with unexpected
identifiers.. Perhaps the temporary variable should be at least
prefixed with an underscore or something.
But if that is ok then perhaps the existing functions in
include/grub/misc.h should be converted.
There is grub_max, grub_abs and grub_div_roundup already.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 21:05 ` Michal Suchanek
@ 2010-02-16 21:14 ` richardvoigt
2010-02-16 23:19 ` Michal Suchanek
2010-02-16 21:53 ` Michal Suchanek
1 sibling, 1 reply; 24+ messages in thread
From: richardvoigt @ 2010-02-16 21:14 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Feb 16, 2010 at 3:05 PM, Michal Suchanek <hramrach@centrum.cz> wrote:
> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> Michal Suchanek wrote:
>>>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>>>> +static inline long
>>>> +grub_min (long x, long y)
>>>> +{
>>>> + if (x > y)
>>>> + return y;
>>>> + else
>>>> + return x;
>>>> +}
>>>> +
>>>>
>>>
>>> I don't see how typeof would be used. As I understand the docs it can
>>> only set types relative to something what is already defined (and in
>>> some cases actually dereference/call it) and there is nothing defined
>>> at the point these functions are declared to copy the type from.
>>>
>> #include <stdio.h>
>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>> b; b = a; a = mytemp ## __LINE__; }
>>
>
> Unlike inlines this pollutes the local namespace with unexpected
> identifiers.. Perhaps the temporary variable should be at least
> prefixed with an underscore or something.
The braces introduce a block and the variable goes out of scope, in
fact there's no need for __LINE__ because of this.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 21:05 ` Michal Suchanek
2010-02-16 21:14 ` richardvoigt
@ 2010-02-16 21:53 ` Michal Suchanek
1 sibling, 0 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 21:53 UTC (permalink / raw)
To: The development of GNU GRUB
On 16 February 2010 22:05, Michal Suchanek <hramrach@centrum.cz> wrote:
> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> Michal Suchanek wrote:
>>>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>>>> +static inline long
>>>> +grub_min (long x, long y)
>>>> +{
>>>> + if (x > y)
>>>> + return y;
>>>> + else
>>>> + return x;
>>>> +}
>>>> +
>>>>
>>>
>>> I don't see how typeof would be used. As I understand the docs it can
>>> only set types relative to something what is already defined (and in
>>> some cases actually dereference/call it) and there is nothing defined
>>> at the point these functions are declared to copy the type from.
>>>
>> #include <stdio.h>
>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>> b; b = a; a = mytemp ## __LINE__; }
>>
>
> Unlike inlines this pollutes the local namespace with unexpected
> identifiers.. Perhaps the temporary variable should be at least
> prefixed with an underscore or something.
>
> But if that is ok then perhaps the existing functions in
> include/grub/misc.h should be converted.
>
> There is grub_max, grub_abs and grub_div_roundup already.
Hmm, we don't have stdio.h in grub.
Perhaps it's no that good idea after all.
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC] Framebuffer rotation patch
2010-02-16 21:14 ` richardvoigt
@ 2010-02-16 23:19 ` Michal Suchanek
0 siblings, 0 replies; 24+ messages in thread
From: Michal Suchanek @ 2010-02-16 23:19 UTC (permalink / raw)
To: The development of GNU GRUB
On 16 February 2010 22:14, richardvoigt@gmail.com
<richardvoigt@gmail.com> wrote:
> On Tue, Feb 16, 2010 at 3:05 PM, Michal Suchanek <hramrach@centrum.cz> wrote:
>> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>> Michal Suchanek wrote:
>>>>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>>>>> +static inline long
>>>>> +grub_min (long x, long y)
>>>>> +{
>>>>> + if (x > y)
>>>>> + return y;
>>>>> + else
>>>>> + return x;
>>>>> +}
>>>>> +
>>>>>
>>>>
>>>> I don't see how typeof would be used. As I understand the docs it can
>>>> only set types relative to something what is already defined (and in
>>>> some cases actually dereference/call it) and there is nothing defined
>>>> at the point these functions are declared to copy the type from.
>>>>
>>> #include <stdio.h>
>>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>>> b; b = a; a = mytemp ## __LINE__; }
>>>
>>
>> Unlike inlines this pollutes the local namespace with unexpected
>> identifiers.. Perhaps the temporary variable should be at least
>> prefixed with an underscore or something.
>
> The braces introduce a block and the variable goes out of scope, in
> fact there's no need for __LINE__ because of this.
It breaks things when you do have a mytemp already:
$ gcc -Wall testdef.c
$ ./a.out
2 1 3 4
$ cat testdef.c
#include <stdio.h>
#define swap(a,b) {typeof (a) mytemp; mytemp = b; b = a; a = mytemp; }
int main( int argc, char ** argv)
{
int x=1, y=2, z=3, mytemp=4;
swap(x,y);
swap(z,mytemp);
return printf("%i %i %i %i\n", x, y, z, mytemp);
}
Thanks
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2010-02-16 23:20 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-28 8:03 [RFC] Framebuffer rotation patch Michal Suchanek
2009-08-29 10:24 ` Michal Suchanek
2010-02-11 2:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-11 10:19 ` Michal Suchanek
2010-02-11 10:52 ` Michal Suchanek
2010-02-11 16:11 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-11 16:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-13 17:29 ` Michal Suchanek
2010-02-15 17:05 ` [RFC] Framebuffer rotation patch, or why 'unsigned' fails us Colin D Bennett
2010-02-15 18:14 ` Michal Suchanek
2010-02-15 18:35 ` Colin D Bennett
2010-02-16 12:35 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 20:53 ` Michal Suchanek
2010-02-16 12:32 ` [RFC] Framebuffer rotation patch Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 15:52 ` Michal Suchanek
2010-02-16 18:03 ` Isaac Dupree
2010-02-16 18:15 ` richardvoigt
2010-02-16 18:47 ` [Off-topic] C++ enums Isaac Dupree
2010-02-16 16:08 ` [RFC] Framebuffer rotation patch Michal Suchanek
2010-02-16 16:21 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-02-16 21:05 ` Michal Suchanek
2010-02-16 21:14 ` richardvoigt
2010-02-16 23:19 ` Michal Suchanek
2010-02-16 21:53 ` Michal Suchanek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.