| Re: [AD] proposal: draw_sprite_*_ex |
[ Thread Index |
Date Index
| More lists.liballeg.org/allegro-developers Archives
]
On 2007-12-27, Milan Mimica <milan.mimica@xxxxxxxxxx> wrote:
> Peter Wang wrote:
> > On 2007-10-07, Milan Mimica <milan.mimica@xxxxxxxxxx> wrote:
> >> DRAW_SPRITE_NO_FLIP | DRAW_SPRITE_H_FLIP Is the only open issue.
> >> Can't we just have DRAW_SPRITE_HV_FLIP and not allow or-ing?
> > I don't remember why I suggesting OR-ing, so I have no problem
> > with that.
> > The patch needs some reformatting.
>
>
> Here is the reformatted and cleaned patch.
>
> Flipping mode flags are:
> /* flipping modes for draw_sprite_ex() */
> #define DRAW_SPRITE_NO_FLIP 0x0
> #define DRAW_SPRITE_H_FLIP 0x1
> #define DRAW_SPRITE_V_FLIP 0x2
> #define DRAW_SPRITE_VH_FLIP 0x3
> #define DRAW_SPRITE_HV_FLIP 0x3
I think having both VH and HV isn't worthwhile. I can imagine searching for
HV_FLIP in the source and getting confused because it only refers to VH_FLIP.
> - No or-ing (at least not between these).
> - DRAW_SPRITE_NO_FLIP could be changed to DRAW_SPRITE_PLAIN but that's a
> minor thing.
> - No docs.
> - The patch is only for non-ASM version. That's the problem.
>
> Should we (in 4.3.10+):
> a) remove the ASM version completely?
Yes.
> b) just make the non-ASM version default?
>
> (b) is a lot easier to do. Although, it doesn't really fix anything. ASM and
> non-ASM would not be *API* compatible this time.
The asm code doesn't need to be removed immediately.
> Index: src/c/cdefs24.h
> ===================================================================
> --- src/c/cdefs24.h (revision 9732)
> +++ src/c/cdefs24.h (working copy)
> @@ -24,6 +24,7 @@
> #define PTR_PER_PIXEL 3
> #define OFFSET_PIXEL_PTR(p,x) ((PIXEL_PTR) (p) + 3 * (x))
> #define INC_PIXEL_PTR(p) ((p) += 3)
> +#define INC_PIXEL_PTR_EX(p,d) ((p) += 3 * d)
> #define DEC_PIXEL_PTR(p) ((p) -= 3)
>
> #define PUT_PIXEL(p,c) bmp_write24((uintptr_t) (p), (c))
> @@ -48,6 +49,7 @@
> #define DLS_BLENDER BLENDER_FUNC
> #define MAKE_DLS_BLENDER(a) _blender_func24
> #define DLS_BLEND(b,a,n) ((*(b))(_blender_col_24, (n), (a)))
> +#define DLSX_BLEND(b,n) ((*(b))(_blender_col_24, (n), _blender_alpha))
Align that with an extra space.
> Index: src/c/cdefs32.h
> ===================================================================
> --- src/c/cdefs32.h (revision 9732)
> +++ src/c/cdefs32.h (working copy)
> @@ -24,6 +24,7 @@
> #define PTR_PER_PIXEL 1
> #define OFFSET_PIXEL_PTR(p,x) ((PIXEL_PTR) (p) + (x))
> #define INC_PIXEL_PTR(p) ((p)++)
> +#define INC_PIXEL_PTR_EX(p,d) ((p) += d)
Maybe name this INC_PIXEL_PTR_N? Ditto elsewhere.
> #define DEC_PIXEL_PTR(p) ((p)--)
>
> #define PUT_PIXEL(p,c) bmp_write32((uintptr_t) (p), (c))
> @@ -49,6 +50,7 @@
> #define DLS_BLENDER BLENDER_FUNC
> #define MAKE_DLS_BLENDER(a) _blender_func32
> #define DLS_BLEND(b,a,n) ((*(b))(_blender_col_32, (n), (a)))
> +#define DLSX_BLEND(b,n) ((*(b))(_blender_col_32, (n), _blender_alpha))
>
> /* Blender for poly_scanline_*_lit. */
> #define PS_BLENDER BLENDER_FUNC
> @@ -75,10 +77,14 @@
> #define FUNC_LINEAR_VLINE _linear_vline32
>
> #define FUNC_LINEAR_DRAW_SPRITE _linear_draw_sprite32
> +#define FUNC_LINEAR_DRAW_SPRITE_EX _linear_draw_sprite_ex32
> #define FUNC_LINEAR_DRAW_256_SPRITE _linear_draw_256_sprite32
> #define FUNC_LINEAR_DRAW_SPRITE_V_FLIP _linear_draw_sprite_v_flip32
> +#define FUNC_LINEAR_DRAW_SPRITE_V_FLIP_EX _linear_draw_sprite_v_flip_ex32
> #define FUNC_LINEAR_DRAW_SPRITE_H_FLIP _linear_draw_sprite_h_flip32
> +#define FUNC_LINEAR_DRAW_SPRITE_H_FLIP_EX _linear_draw_sprite_h_flip_ex32
> #define FUNC_LINEAR_DRAW_SPRITE_VH_FLIP _linear_draw_sprite_vh_flip32
> +#define FUNC_LINEAR_DRAW_SPRITE_VH_FLIP_EX _linear_draw_sprite_vh_flip_ex32
> #define FUNC_LINEAR_DRAW_TRANS_SPRITE _linear_draw_trans_sprite32
> #define FUNC_LINEAR_DRAW_TRANS_RGBA_SPRITE _linear_draw_trans_rgba_sprite32
> #define FUNC_LINEAR_DRAW_LIT_SPRITE _linear_draw_lit_sprite32
> Index: src/c/cdefs16.h
> ===================================================================
> --- src/c/cdefs16.h (revision 9732)
> +++ src/c/cdefs16.h (working copy)
> @@ -24,6 +24,7 @@
> #define PTR_PER_PIXEL 1
> #define OFFSET_PIXEL_PTR(p,x) ((PIXEL_PTR) (p) + (x))
> #define INC_PIXEL_PTR(p) ((p)++)
> +#define INC_PIXEL_PTR_EX(p,d) ((p) += d)
> #define DEC_PIXEL_PTR(p) ((p)--)
>
> #define PUT_PIXEL(p,c) bmp_write16((uintptr_t) (p), (c))
> @@ -37,7 +38,7 @@
>
> /* Blender for putpixel (DRAW_MODE_TRANS). */
> #define PP_BLENDER BLENDER_FUNC
> -#define MAKE_PP_BLENDER(c) _blender_func16
> +#define MAKE_PP_BLENDER(c) (!_blender_func16 && (al_assert(__FILE__,__LINE__), 0), _blender_func16)
That's a bit ugly :-P
> #define PP_BLEND(b,o,n) ((*(b))((n), (o), _blender_alpha))
>
> /* Blender for draw_trans_*_sprite. */
> @@ -49,6 +50,7 @@
> #define DLS_BLENDER BLENDER_FUNC
> #define MAKE_DLS_BLENDER(a) _blender_func16
> #define DLS_BLEND(b,a,n) ((*(b))(_blender_col_16, (n), (a)))
> +#define DLSX_BLEND(b,n) ((*(b))(_blender_col_16, (n), _blender_alpha))
Alignment.
> Index: src/c/cspr.h
> ===================================================================
> --- src/c/cspr.h (revision 9732)
> +++ src/c/cspr.h (working copy)
> @@ -18,6 +18,138 @@
> #ifndef __bma_cspr_h
> #define __bma_cspr_h
>
Please add a comment to describe this a bit.
> +void FUNC_LINEAR_DRAW_SPRITE_EX(BITMAP * dst, BITMAP * src, int dx, int dy, int mode, int flip)
> +{
> + int x, y, w, h;
> + int x_dir = 1, y_dir = 1;
> + int dxbeg, dybeg;
> + int sxbeg, sybeg;
> + DLS_BLENDER lit_blender;
> + DTS_BLENDER trans_blender;
> +
> + ASSERT(dst);
> + ASSERT(src);
> +
> + if (flip == DRAW_SPRITE_V_FLIP) {
> + y_dir = -1;
> + }
> + if (flip == DRAW_SPRITE_H_FLIP) {
> + x_dir = -1;
> + }
> + if (flip == DRAW_SPRITE_VH_FLIP) {
> + y_dir = -1;
> + x_dir = -1;
> + }
> +
> + if (dst->clip) {
> + int tmp;
> +
> + tmp = dst->cl - dx;
> + sxbeg = ((tmp < 0) ? 0 : tmp);
You can use MAX().
> + dxbeg = sxbeg + dx;
> +
> + tmp = dst->cr - dx;
> + w = ((tmp > src->w) ? src->w : tmp) - sxbeg;
MIN()
> + if (w <= 0)
> + return;
Expand the tab there.
> +
> + if (flip == DRAW_SPRITE_H_FLIP || flip == DRAW_SPRITE_VH_FLIP) {
> + /* use backward drawing onto dst */
> + sxbeg = src->w - (sxbeg + w);
> + dxbeg += w - 1;
> + }
That block is 4-space indented.
> +
> + tmp = dst->ct - dy;
> + sybeg = ((tmp < 0) ? 0 : tmp);
> + dybeg = sybeg + dy;
> +
> + tmp = dst->cb - dy;
> + h = ((tmp > src->h) ? src->h : tmp) - sybeg;
> + if (h <= 0)
> + return;
> +
> + if (flip == DRAW_SPRITE_V_FLIP || flip == DRAW_SPRITE_VH_FLIP) {
> + /* use backward drawing onto dst */
> + sybeg = src->h - (sybeg + h);
> + dybeg += h - 1;
> + }
Ditto.
> + }
> + else {
> + w = src->w;
> + h = src->h;
> + sxbeg = 0;
> + sybeg = 0;
> + dxbeg = dx;
> + if (flip == DRAW_SPRITE_H_FLIP || flip == DRAW_SPRITE_VH_FLIP) {
> + dxbeg = dx + w - 1;
> + }
> + dybeg = dy;
> + if (flip == DRAW_SPRITE_V_FLIP || flip == DRAW_SPRITE_VH_FLIP) {
> + dybeg = dy + h - 1;
> + }
> + }
> +
> + lit_blender = MAKE_DLS_BLENDER(0);
> + trans_blender = MAKE_DTS_BLENDER();
> +
> + if (dst->id & (BMP_ID_VIDEO | BMP_ID_SYSTEM)) {
> + bmp_select(dst);
> +
> + for (y = 0; y < h; y++) {
> + PIXEL_PTR s = OFFSET_PIXEL_PTR(src->line[sybeg + y], sxbeg);
> +
> + /* flipped if y_dir is -1 */
> + PIXEL_PTR d = OFFSET_PIXEL_PTR(bmp_write_line(dst, dybeg + y * y_dir), dxbeg);
> +
Indentation below is screwed due to tabs. Also, reformat the switch in the
usual way.
> + /* d is incremented by x_dir, -1 if flipped */
> + for (x = w - 1; x >= 0; INC_PIXEL_PTR(s), INC_PIXEL_PTR_EX(d,x_dir), x--) {
> + unsigned long c = GET_MEMORY_PIXEL(s);
> + if (!IS_SPRITE_MASK(src, c)) {
> + switch(mode) {
> + case DRAW_SPRITE_NORMAL : break;
> + case DRAW_SPRITE_LIT : {
> + c = DLSX_BLEND(lit_blender, c);
> + break;
> + }
> + case DRAW_SPRITE_TRANS : {
> + c = DTS_BLEND(trans_blender, GET_PIXEL(d), c);
> + break;
> + }
> + }
> + PUT_PIXEL(d, c);
> + }
> + }
> + }
> +
> + bmp_unwrite_line(dst);
> + }
> + else {
> + for (y = 0; y < h; y++) {
> + PIXEL_PTR s = OFFSET_PIXEL_PTR(src->line[sybeg + y], sxbeg);
> + PIXEL_PTR d = OFFSET_PIXEL_PTR(bmp_write_line(dst, dybeg + y * y_dir), dxbeg);
> +
> + for (x = w - 1; x >= 0; INC_PIXEL_PTR(s), INC_PIXEL_PTR_EX(d,x_dir), x--) {
> + unsigned long c = GET_MEMORY_PIXEL(s);
> + if (!IS_SPRITE_MASK(src, c)) {
> + switch(mode) {
> + case DRAW_SPRITE_NORMAL : break;
> + case DRAW_SPRITE_LIT : {
> + c = DLSX_BLEND(lit_blender, c);
> + break;
> + }
> + case DRAW_SPRITE_TRANS : {
> + c = DTS_BLEND(trans_blender, GET_PIXEL(d), c);
> + break;
> + }
> + }
> + PUT_MEMORY_PIXEL(d, c);
> + }
> + }
> + }
Ditto.
The same problems occur a few more times in the patch.
> Index: src/x/xvtable.c
> ===================================================================
> --- src/x/xvtable.c (revision 9732)
> +++ src/x/xvtable.c (working copy)
> @@ -33,10 +33,14 @@
> static void _xwin_hline(BITMAP *dst, int dx1, int dy, int dx2, int color);
> static void _xwin_rectfill(BITMAP *dst, int dx1, int dy1, int dx2, int dy2, int color);
> static void _xwin_draw_sprite(BITMAP *dst, BITMAP *src, int dx, int dy);
> +static void _xwin_draw_sprite_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode, int flip);
> static void _xwin_draw_256_sprite(BITMAP *dst, BITMAP *src, int dx, int dy);
> static void _xwin_draw_sprite_v_flip(BITMAP *dst, BITMAP *src, int dx, int dy);
> +static void _xwin_draw_sprite_v_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode);
> static void _xwin_draw_sprite_h_flip(BITMAP *dst, BITMAP *src, int dx, int dy);
> +static void _xwin_draw_sprite_h_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode);
> static void _xwin_draw_sprite_vh_flip(BITMAP *dst, BITMAP *src, int dx, int dy);
> +static void _xwin_draw_sprite_vh_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode);
> static void _xwin_draw_trans_sprite(BITMAP *dst, BITMAP *src, int dx, int dy);
> static void _xwin_draw_trans_rgba_sprite(BITMAP *dst, BITMAP *src, int dx, int dy);
> static void _xwin_draw_lit_sprite(BITMAP *dst, BITMAP *src, int dx, int dy, int color);
> @@ -233,10 +237,14 @@
> vtable->hfill = _xwin_hline;
> vtable->rectfill = _xwin_rectfill;
> vtable->draw_sprite = _xwin_draw_sprite;
> + vtable->draw_sprite_ex = _xwin_draw_sprite_ex;
> vtable->draw_256_sprite = _xwin_draw_256_sprite;
> vtable->draw_sprite_v_flip = _xwin_draw_sprite_v_flip;
> + vtable->draw_sprite_v_flip_ex = _xwin_draw_sprite_v_flip_ex;
> vtable->draw_sprite_h_flip = _xwin_draw_sprite_h_flip;
> + vtable->draw_sprite_h_flip_ex = _xwin_draw_sprite_h_flip_ex;
> vtable->draw_sprite_vh_flip = _xwin_draw_sprite_vh_flip;
> + vtable->draw_sprite_vh_flip_ex = _xwin_draw_sprite_vh_flip_ex;
> vtable->draw_trans_sprite = _xwin_draw_trans_sprite;
> vtable->draw_trans_rgba_sprite = _xwin_draw_trans_rgba_sprite;
> vtable->draw_lit_sprite = _xwin_draw_lit_sprite;
> @@ -569,8 +577,26 @@
> _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> }
>
> +/* _xwin_draw_sprite_ex:
> + * Wrapper for draw_sprite_ex.
> + */
> +static void _xwin_draw_sprite_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode, int flip)
> +{
> + int dxbeg, dybeg, w, h;
>
> + if (_xwin_in_gfx_call) {
> + _xwin_vtable.draw_sprite_ex(dst, src, dx, dy, mode, flip);
> + return;
> + }
>
> + CLIP_BOX(dst, dxbeg, dybeg, w, h, dx, dy, src->w, src->h)
> +
> + _xwin_in_gfx_call = 1;
> + _xwin_vtable.draw_sprite_ex(dst, src, dx, dy, mode, flip);
> + _xwin_in_gfx_call = 0;
> + _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> +}
> +
> /* _xwin_draw_256_sprite:
> * Wrapper for draw_256_sprite.
> */
> @@ -613,8 +639,28 @@
> _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> }
>
> +/* _xwin_draw_sprite_v_flip:
> + * Wrapper for draw_sprite_v_flip.
> + */
> +static void _xwin_draw_sprite_v_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode)
> +{
> + int dxbeg, dybeg, w, h;
>
> + if (_xwin_in_gfx_call) {
> + _xwin_vtable.draw_sprite_v_flip_ex(dst, src, dx, dy, mode);
> + return;
> + }
>
> + CLIP_BOX(dst, dxbeg, dybeg, w, h, dx, dy, src->w, src->h)
> +
> + _xwin_in_gfx_call = 1;
> + _xwin_vtable.draw_sprite_v_flip_ex(dst, src, dx, dy, mode);
> + _xwin_in_gfx_call = 0;
> + _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> +}
> +
> +
> +
> /* _xwin_draw_sprite_h_flip:
> * Wrapper for draw_sprite_h_flip.
> */
> @@ -635,8 +681,26 @@
> _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> }
>
> +/* _xwin_draw_sprite_h_flip_ex:
> + * Wrapper for draw_sprite_h_flip_ex.
> + */
> +static void _xwin_draw_sprite_h_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode)
> +{
> + int dxbeg, dybeg, w, h;
>
> + if (_xwin_in_gfx_call) {
> + _xwin_vtable.draw_sprite_h_flip_ex(dst, src, dx, dy, mode);
> + return;
> + }
>
> + CLIP_BOX(dst, dxbeg, dybeg, w, h, dx, dy, src->w, src->h)
> +
> + _xwin_in_gfx_call = 1;
> + _xwin_vtable.draw_sprite_h_flip_ex(dst, src, dx, dy, mode);
> + _xwin_in_gfx_call = 0;
> + _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> +}
> +
> /* _xwin_draw_sprite_vh_flip:
> * Wrapper for draw_sprite_vh_flip.
> */
> @@ -657,8 +721,26 @@
> _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> }
>
> +/* _xwin_draw_sprite_vh_flip_ex:
> + * Wrapper for draw_sprite_vh_flip_ex.
> + */
> +static void _xwin_draw_sprite_vh_flip_ex(BITMAP *dst, BITMAP *src, int dx, int dy, int mode)
> +{
> + int dxbeg, dybeg, w, h;
>
> + if (_xwin_in_gfx_call) {
> + _xwin_vtable.draw_sprite_vh_flip_ex(dst, src, dx, dy, mode);
> + return;
> + }
>
> + CLIP_BOX(dst, dxbeg, dybeg, w, h, dx, dy, src->w, src->h)
> +
> + _xwin_in_gfx_call = 1;
> + _xwin_vtable.draw_sprite_vh_flip_ex(dst, src, dx, dy, mode);
> + _xwin_in_gfx_call = 0;
> + _xwin_update_video_bitmap(dst, dxbeg, dybeg, w, h);
> +}
> +
> /* _xwin_draw_trans_sprite:
> * Wrapper for draw_trans_sprite.
> */
> Index: examples/extrans2.c
> ===================================================================
> --- examples/extrans2.c (revision 0)
> +++ examples/extrans2.c (revision 0)
> @@ -0,0 +1,241 @@
> +/*
> + * Example program for the Allegro library, by Jon Rafkind.
> + *
> + * This program demonstrates how to draw trans and lit sprites and flip them
> + * at the same time, using draw_trans_sprite_ex() function.
> + * It displays several images moving around using different drawing modes
> + * while you can press space key to change the flipping mode.
> + */
> +
> +#include <stdio.h>
> +
> +#include <allegro.h>
> +
> +#define UP 0
> +#define LEFT 1
> +#define DOWN 2
> +#define RIGHT 3
> +
> +/* define the number of sprites that will be displayed */
> +#define SPRITE_COUNT 20
> +
> +
> +/* define our sprite */
> +typedef struct {
> + int x;
> + int y;
> +
> + /* 0 = up, 1 = left, 2 = down, 3 = right */
> + int direction;
Might as well use an enum DIR and delete the comment.
> +
> + /* what order to draw this sprite at, lower before higher */
> + int level;
> +
> + /* SPRITE_NORMAL, SPRITE_TRANS, or SPRITE_LIT */
> + int draw_type;
> +} sprite;
Name this SPRITE to avoid confusion with a variable named sprite.
> +
> +
> +/* puts the sprite somewhere on the screen and sets its initial state */
> +void setup_sprite(sprite *s, int i)
> +{
> + switch (i % 4) {
> + case 0 : {
> + s->x = -100 + i * 50;
> + s->y = 40 + (i % 3) * 100;
> + s->direction = RIGHT;
> + s->draw_type = DRAW_SPRITE_NORMAL;
> + s->level = 0;
> + break;
> + }
> + case 1 : {
> + s->x = 640 + 100 - i * 70;
> + s->y = 50 + (i % 3) * 110;
> + s->direction = LEFT;
> + s->draw_type = DRAW_SPRITE_TRANS;
> + s->level = 1;
> + break;
> + }
> + case 2 : {
> + s->x = 90 + (i % 3) * 200;
> + s->y = -100 + i * 70;
> + s->direction = DOWN;
> + s->draw_type = DRAW_SPRITE_LIT;
> + s->level = 2;
> + break;
> + }
> + case 3 : {
> + s->x = 50 + (i % 3) * 200;
> + s->y = 480 + 100 - i * 70;
> + s->direction = UP;
> + s->draw_type = DRAW_SPRITE_TRANS;
> + s->level = 3;
> + break;
> + }
> + }
> +}
> +
> +
> +/* used by sort function to compare sprites */
> +int compare(AL_CONST void * a, AL_CONST void * b) {
Name this compare_sprite. Move the opening brace down, here and elsewhere.
> + sprite * s1 = (sprite *)a;
> + sprite * s2 = (sprite *)b;
> + if (s1->level < s2->level) {
> + return -1;
> + }
> + if (s1->level > s2->level) {
> + return 1;
> + }
> + return 0;
> +}
You can write:
return s1->level - s2->level;
Peter