graphics/: Correct logic for copy of bit maps with resolution less than 8 bits from the per-window framebuffer to the device.

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
   ^^^^^^^^^^^^
 

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"

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] =
+  struct nxgl_point_s origin;
-  {
+  unsigned int bpp;
-    (FAR const void *)wnd->fbmem
-  };
-  struct nxgl_point_s origin =
-  {
-    0, 0
-  };
 
   /* 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.
    */
 

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
+  /* Get the source of address of the moved rectangle in the framebuffer. */
-   * 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.
-   */
 
   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
+  /* For resolutions less than 8-bits, the starting pixel will be contained
-   * device memory.
+   * 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);

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] =
+      struct nxgl_point_s origin;
-      {
+      unsigned int bpp;
-        (FAR const void *)wnd->fbmem
-      };
-      struct nxgl_point_s origin =
-      {
-        0, 0
-      };
 
       /* 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);

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) \

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;

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,
+  ret = nx_constructwindow(handle, (NXWINDOW)&fwnd->wnd,
-                           NULL);
+                           flags | NXBE_WINDOW_FRAMED, &g_nxtkcb, NULL);
   if (ret < 0)
     {
       /* An error occurred, the window has been freed */

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))