From 10b73fde9dc46a26369301c7be96f15a2c11de20 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 18 Mar 2019 09:44:38 -0600 Subject: [PATCH] graphics/: Correct logic for copy of bit maps with resolution less than 8 bits from the per-window framebuffer to the device. --- TODO | 24 +++++++++++- graphics/Kconfig | 7 +++- graphics/nxbe/nxbe_filltrapezoid.c | 61 +++++++++++++++++++++++++----- graphics/nxbe/nxbe_move.c | 60 +++++++++++++++++++++-------- graphics/nxmu/nxmu_redrawreq.c | 57 ++++++++++++++++++++++++---- include/nuttx/nx/nxbe.h | 22 +++++++++-- libs/libnx/nxmu/nx_openwindow.c | 2 +- libs/libnx/nxtk/nxtk_openwindow.c | 6 +-- libs/libnx/nxtk/nxtk_setsize.c | 3 -- 9 files changed, 196 insertions(+), 46 deletions(-) diff --git a/TODO b/TODO index 3f1e0222cc..b9a845141b 100644 --- a/TODO +++ b/TODO @@ -1,4 +1,4 @@ -NuttX TODO List (Last updated March 15, 2019) +NuttX TODO List (Last updated March 18, 2019) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This file summarizes known NuttX bugs, limitations, inconsistencies with @@ -24,7 +24,7 @@ nuttx/: (2) Other drivers (drivers/) (9) Libraries (libs/libc/, libs/libm/) (12) File system/Generic drivers (fs/, drivers/) - (9) Graphics Subsystem (graphics/) + (10) Graphics Subsystem (graphics/) (1) Build system / Toolchains (3) Linux/Cywgin simulation (arch/sim) (5) ARM (arch/arm/) @@ -2450,6 +2450,26 @@ o Graphics Subsystem (graphics/) depth. If such a beast ever shows up, then this priority would be higher. + Title: INCOMPLATE PLANAR COLOR SUPPORT + Description: The original NX design included support for planar colors, + i.e,. for devices that provide separate framebuffers for each + color component. Planar graphics hard was common some years + back but is rarely encountered today. In fact, I am not aware + of any MCU that implements planar framebuffers. + + Support for planar colors is, however, unverified and + incomplete. In fact, many recent changes explicitly assume a + single color plane: Planar colors are specified by a array + of components; some recent logic uses only component [0], + ignoring the possible existence of other color componet frames. + + Completely removing planar color support is one reasonable + options; it is not likely that NuttX will encounter planar + color hardware and this would greatly simplify the logic and + eliminate inconsistencies in the immplementation. + Status: Open + Priority: Low. There is no problem other than one of aesthetics. + o Build system ^^^^^^^^^^^^ diff --git a/graphics/Kconfig b/graphics/Kconfig index 1221d89fa5..aea842c943 100644 --- a/graphics/Kconfig +++ b/graphics/Kconfig @@ -62,7 +62,12 @@ config NX_RAMBACKED window at the expense of increased memory usage. Redraw requests in other cases are also suppressed: Changes to window - position, size, etc. + position, size, etc. As a consequence, some manual updates will be + required when certain events occurr (like removing a toolbar from a + window) + + NOTE: A significant amount of RAM, usually external SDRAM, is + required to run this demo. config NX_BGCOLOR hex "Initial background color" diff --git a/graphics/nxbe/nxbe_filltrapezoid.c b/graphics/nxbe/nxbe_filltrapezoid.c index 313182414e..82adbd5190 100644 --- a/graphics/nxbe/nxbe_filltrapezoid.c +++ b/graphics/nxbe/nxbe_filltrapezoid.c @@ -179,16 +179,11 @@ static inline void nxbe_filltrapezoid_pwfb(FAR struct nxbe_window_s *wnd, FAR const struct nxgl_trapezoid_s *trap, nxgl_mxpixel_t color[CONFIG_NX_NPLANES]) { + FAR const void *src[CONFIG_NX_NPLANES]; struct nxgl_trapezoid_s reltrap; struct nxgl_rect_s relbounds; - FAR const void *src[CONFIG_NX_NPLANES] = - { - (FAR const void *)wnd->fbmem - }; - struct nxgl_point_s origin = - { - 0, 0 - }; + struct nxgl_point_s origin; + unsigned int bpp; /* Both the rectangle that we receive here are in abolute device * coordinates. We need to restore both to windows relative coordinates. @@ -207,7 +202,55 @@ static inline void nxbe_filltrapezoid_pwfb(FAR struct nxbe_window_s *wnd, wnd->be->plane[0].pwfb.filltrapezoid(wnd, &reltrap, &relbounds, color[0]); - /* Copy the portion of the per-window framebuffer in the bounding box + /* Get the source of address of the trapezoid bounding box in the + * framebuffer. + */ + + bpp = wnd->be->plane[0].pinfo.bpp; + src[0] = (FAR const void *) + ((FAR uint8_t *)wnd->fbmem + + relbounds.pt1.y * wnd->stride + + ((bpp * relbounds.pt1.x) >> 3)); + + /* For resolutions less than 8-bits, the starting pixel will be contained + * in the byte pointed to by src[0]but may not be properly aligned for + * the transfer. We fix this by modifying the origin. + */ + + origin.x = relbounds.pt1.x; + origin.y = relbounds.pt1.y; + + switch (bpp) + { +#ifndef CONFIG_NX_DISABLE_1BPP + case 1: /* 1 bit per pixel */ + { + origin.x &= ~7; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_2BPP + case 2: /* 2 bits per pixel */ + { + origin.x &= ~3; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_4BPP + case 4: /* 4 bits per pixel */ + { + origin.x &= ~1; + } + break; +#endif + + default: + break; + } + +/* Copy the portion of the per-window framebuffer in the bounding box * to the device graphics memory. */ diff --git a/graphics/nxbe/nxbe_move.c b/graphics/nxbe/nxbe_move.c index 2e3a3dd903..9bd077ec3a 100644 --- a/graphics/nxbe/nxbe_move.c +++ b/graphics/nxbe/nxbe_move.c @@ -294,14 +294,11 @@ static inline void nxbe_move_pwfb(FAR struct nxbe_window_s *wnd, FAR const struct nxgl_rect_s *rect, FAR const struct nxgl_point_s *offset) { + FAR const void *src[CONFIG_NX_NPLANES]; struct nxgl_point_s destpos; + struct nxgl_point_s origin; struct nxgl_rect_s srcrect; struct nxgl_rect_s destrect; - FAR const void *src[CONFIG_NX_NPLANES]; - FAR const struct nxgl_point_s origin = - { - 0, 0 - }; unsigned int bpp; /* The rectangle that we receive here is in abolute device coordinates. We @@ -330,23 +327,54 @@ static inline void nxbe_move_pwfb(FAR struct nxbe_window_s *wnd, nxgl_rectoffset(&destrect, &srcrect, offset->x, offset->y); - /* Update the physical device by just copying the rectangle from the - * framebuffer to the device graphics memory. - * - * Get the source of address of the moved rectangle in the framebuffer. - * REVISIT: For resolutions less than 8-bits, the starting pixel will - * be contained in the byte pointed to by src[0]but may not be properly - * aligned for the transfer. - */ + /* Get the source of address of the moved rectangle in the framebuffer. */ bpp = wnd->be->plane[0].pinfo.bpp; - src[0] = (FAR void *) + src[0] = (FAR const void *) ((FAR uint8_t *)wnd->fbmem + destrect.pt1.y * wnd->stride + ((bpp * destrect.pt1.x) >> 3)); - /* Then transfer the rectangle to the destination rectangle in the - * device memory. + /* For resolutions less than 8-bits, the starting pixel will be contained + * in the byte pointed to by src[0]but may not be properly aligned for + * the transfer. We fix this by modifying the origin. + */ + + origin.x = destrect.pt1.x; + origin.y = destrect.pt1.y; + + switch (bpp) + { +#ifndef CONFIG_NX_DISABLE_1BPP + case 1: /* 1 bit per pixel */ + { + origin.x &= ~7; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_2BPP + case 2: /* 2 bits per pixel */ + { + origin.x &= ~3; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_4BPP + case 4: /* 4 bits per pixel */ + { + origin.x &= ~1; + } + break; +#endif + + default: + break; + } + + /* Update the physical device by just copying the rectangle from the + * framebuffer to the destination rectangle device graphics memory. */ nxbe_bitmap_dev(wnd, &destrect, src, &origin, wnd->stride); diff --git a/graphics/nxmu/nxmu_redrawreq.c b/graphics/nxmu/nxmu_redrawreq.c index 00e84b92a6..4f5b76ca0f 100644 --- a/graphics/nxmu/nxmu_redrawreq.c +++ b/graphics/nxmu/nxmu_redrawreq.c @@ -69,21 +69,62 @@ void nxmu_redrawreq(FAR struct nxbe_window_s *wnd, if (NXBE_ISRAMBACKED(wnd)) { + FAR const void *src[CONFIG_NX_NPLANES]; struct nxgl_rect_s wndrect; - FAR const void *src[CONFIG_NX_NPLANES] = - { - (FAR const void *)wnd->fbmem - }; - struct nxgl_point_s origin = - { - 0, 0 - }; + struct nxgl_point_s origin; + unsigned int bpp; /* Put the rectangle back relative to the window */ nxgl_rectoffset(&wndrect, rect, -wnd->bounds.pt1.x, -wnd->bounds.pt1.y); + /* Get the source of address of the rectangle in the framebuffer. */ + + bpp = wnd->be->plane[0].pinfo.bpp; + src[0] = (FAR const void *) + ((FAR uint8_t *)wnd->fbmem + + wndrect.pt1.y * wnd->stride + + ((bpp * wndrect.pt1.x) >> 3)); + + /* For resolutions less than 8-bits, the starting pixel will be + * contained in the byte pointed to by src[0]but may not be properly + * aligned for the transfer. We fix this by modifying the origin. + */ + + origin.x = wndrect.pt1.x; + origin.y = wndrect.pt1.y; + + switch (bpp) + { +#ifndef CONFIG_NX_DISABLE_1BPP + case 1: /* 1 bit per pixel */ + { + origin.x &= ~7; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_2BPP + case 2: /* 2 bits per pixel */ + { + origin.x &= ~3; + } + break; +#endif + +#ifndef CONFIG_NX_DISABLE_4BPP + case 4: /* 4 bits per pixel */ + { + origin.x &= ~1; + } + break; +#endif + + default: + break; + } + /* And render the bitmap */ nxbe_bitmap_dev(wnd, &wndrect, src, &origin, wnd->stride); diff --git a/include/nuttx/nx/nxbe.h b/include/nuttx/nx/nxbe.h index 806ac7adc4..741cdd25df 100644 --- a/include/nuttx/nx/nxbe.h +++ b/include/nuttx/nx/nxbe.h @@ -72,14 +72,23 @@ #define NXBE_WINDOW_BLOCKED (1 << 0) /* The window is blocked and will not * receive further input. */ -#define NXBE_WINDOW_RAMBACKED (1 << 1) /* Window is backed by a framebuffer */ +#define NXBE_WINDOW_FRAMED (1 << 1) /* Framed (NxTK) Window */ +#define NXBE_WINDOW_RAMBACKED (1 << 2) /* Window is backed by a framebuffer */ + +/* Valid user flags for different window types */ #ifdef CONFIG_NX_RAMBACKED -# define NXBE_WINDOW_USER NXBE_WINDOW_RAMBACKED +# define NX_WINDOW_USER NXBE_WINDOW_RAMBACKED +# define NXTK_WINDOW_USER (NXBE_WINDOW_FRAMED | NXBE_WINDOW_RAMBACKED) +# define NXBE_WINDOW_USER (NXBE_WINDOW_FRAMED | NXBE_WINDOW_RAMBACKED) #else -# define NXBE_WINDOW_USER 0 +# define NX_WINDOW_USER 0 +# define NXTK_WINDOW_USER NXBE_WINDOW_FRAMED +# define NXBE_WINDOW_USER NXBE_WINDOW_FRAMED #endif +/* Helpful flag macros */ + #define NXBE_ISBLOCKED(wnd) \ (((wnd)->flags & NXBE_WINDOW_BLOCKED) != 0) #define NXBE_SETBLOCKED(wnd) \ @@ -87,6 +96,13 @@ #define NXBE_CLRBLOCKED(wnd) \ do { (wnd)->flags &= ~NXBE_WINDOW_BLOCKED; } while (0) +#define NXBE_ISFRAMED(wnd) \ + (((wnd)->flags & NXBE_WINDOW_FRAMED) != 0) +#define NXBE_SETFRAMED(wnd) \ + do { (wnd)->flags |= NXBE_WINDOW_FRAMED; } while (0) +#define NXBE_CLRFRAMED(wnd) \ + do { (wnd)->flags &= ~NXBE_WINDOW_FRAMED; } while (0) + #define NXBE_ISRAMBACKED(wnd) \ (((wnd)->flags & NXBE_WINDOW_RAMBACKED) != 0) #define NXBE_SETRAMBACKED(wnd) \ diff --git a/libs/libnx/nxmu/nx_openwindow.c b/libs/libnx/nxmu/nx_openwindow.c index 1b16265d57..44e2e1c6d0 100644 --- a/libs/libnx/nxmu/nx_openwindow.c +++ b/libs/libnx/nxmu/nx_openwindow.c @@ -80,7 +80,7 @@ NXWINDOW nx_openwindow(NXHANDLE handle, uint8_t flags, int ret; #ifdef CONFIG_DEBUG_FEATURES - if (!handle || !cb) + if (handle == NULL || cb == NULL || (flags & ~NX_WINDOW_USER) != 0) { set_errno(EINVAL); return NULL; diff --git a/libs/libnx/nxtk/nxtk_openwindow.c b/libs/libnx/nxtk/nxtk_openwindow.c index 1be1ee38e1..73158b1401 100644 --- a/libs/libnx/nxtk/nxtk_openwindow.c +++ b/libs/libnx/nxtk/nxtk_openwindow.c @@ -109,7 +109,7 @@ NXTKWINDOW nxtk_openwindow(NXHANDLE handle, uint8_t flags, int ret; #ifdef CONFIG_DEBUG_FEATURES - if (handle == NULL || cb == NULL) + if (handle == NULL || cb == NULL || (flags & ~NXTK_WINDOW_USER) != 0) { set_errno(EINVAL); return NULL; @@ -134,8 +134,8 @@ NXTKWINDOW nxtk_openwindow(NXHANDLE handle, uint8_t flags, /* Then let nx_constructwindow do the rest */ - ret = nx_constructwindow(handle, (NXWINDOW)&fwnd->wnd, flags, &g_nxtkcb, - NULL); + ret = nx_constructwindow(handle, (NXWINDOW)&fwnd->wnd, + flags | NXBE_WINDOW_FRAMED, &g_nxtkcb, NULL); if (ret < 0) { /* An error occurred, the window has been freed */ diff --git a/libs/libnx/nxtk/nxtk_setsize.c b/libs/libnx/nxtk/nxtk_setsize.c index 7afa291a50..de246dcaa8 100644 --- a/libs/libnx/nxtk/nxtk_setsize.c +++ b/libs/libnx/nxtk/nxtk_setsize.c @@ -99,9 +99,6 @@ int nxtk_setsize(NXTKWINDOW hfwnd, FAR const struct nxgl_size_s *size) * callbacks. Normally the frame is updated with every redraw callback. * However, as a minimum, the frame only has to but updated after the * window or toolbar sizes change. - * - * REVISIT: Since this works for RAM backed windows, it should work for - * all windows. Why not simplify? */ if (ret >= 0 && NXBE_ISRAMBACKED(&fwnd->wnd))