PyramidFill: Fixed bug around Flagtile class

Fixed bugs which occasionaly fills wrong pixel areas.
It came from wrong initializing codes of m_pixcnt member of Flagtile.
And, there is wrong pixel-matching code at flagtile_flood_fill,
which refers same pixels of around tile border over and over again.
As a result, it erases some flood-fill spillout seeds.

Also, add readonly parameter for lock() method of
Flagtile.

lib/pyramiddefine.hpp

@@ -56,6 +56,12 @@
 // For Flagtile class. to get progress-level ptr from
 #define BUF_PTR(l, x, y) (m_buf + m_buf_offsets[(l)] + ((y) * PYRAMID_TILE_SIZE((l)) + (x)))
 
+// For Flagtile class. pixel-counter buffer size, in bytes.
+#define FLAGTILE_CNT_SIZE ((PIXEL_MAX+1) * (MAX_PYRAMID+1) * sizeof(uint16_t))
+
+// For Flagtile class. to get pixel-counter ptr from
+#define CNT_PTR(l, p) (m_pixcnt + ((l) * (PIXEL_MAX+1)) + ((p) & PIXEL_MASK))
+
 // XXX PIXEL_ Constants.
 //
 // PIXEL_ values are not bitwise flag. They are just number.

lib/pyramidfill.cpp

@@ -636,35 +636,47 @@ Flagtile::fill(const uint8_t val)
     // Fill only equal and above of assigned level.
 
     memset(m_buf, val, FLAGTILE_BUF_SIZE);
-    memset(m_pixcnt, 0, sizeof(uint16_t) * PIXEL_MAX * MAX_PYRAMID);
-    for(int l=0; l < MAX_PYRAMID; l++) {
+    memset(m_pixcnt, 0, sizeof(uint16_t) * PIXEL_MAX * (MAX_PYRAMID + 1));
+
+    for(int l=0; l <= MAX_PYRAMID; l++) {
         int tile_size = PYRAMID_TILE_SIZE(l);
-        m_pixcnt[l][val] = tile_size * tile_size;
+        m_pixcnt[l][val & PIXEL_MASK] = tile_size * tile_size;
     }
 }
 
 /**
 * @to_nparray
 * Get internal m_buf buffer as numpy.array
 *
+* @param readonly Swear that requested object is used as read-only.
 * @return numpy array object.
 *
 * You can get and manipulate m_buf buffer pixel, via numpy API.
 * That numpy array is just `view`, and you must discard(or just
-* stop using) it before this Flagtile instance is released.
+* stop using) view-object before this Flagtile instance is released.
+*
+* Also, you can `swear` that view is used as readonly object.
+* Although, actually you are still enable to change numpy-buffer flag and
+* write change its contents, do not do it. 
 *
 * Use this method carefully.
 */
 PyObject*
-Flagtile::lock()
+Flagtile::lock(const bool readonly)
 {
 #ifdef HEAVY_DEBUG
     assert(m_npbuf == NULL); 
 #endif
-    m_statflag |= COUNT_DIRTY;
     npy_intp dim = FLAGTILE_BUF_SIZE;
     m_npbuf = PyArray_SimpleNewFromData(1, &dim, NPY_UINT8, m_buf);
     Py_INCREF(m_npbuf);
+
+    if (readonly) {
+        PyArray_CLEARFLAGS((PyArrayObject*)m_npbuf, NPY_ARRAY_WRITEABLE);
+    }
+    else {
+        m_statflag |= COUNT_DIRTY;
+    }
     return m_npbuf;
 }
 
@@ -684,11 +696,11 @@ Flagtile::unlock(PyObject *nparray)
     // Assert strictly, even not in HEAVY_DEBUG.
     assert(nparray == m_npbuf); 
 
-    // COUNT_DIRTY statflag should not be cleared here.
-    // To save processing time, pixel-count update 
-    // should be done on-demand.
-    // And, in some case (e.g. frame-bbox trimming, after all pixel decided)
-    // we might not need pixel count anymore.
+    if (!PyArray_ISWRITEABLE((PyArrayObject*)m_npbuf)) {
+        // View object might be changed to writable one. 
+        // Force counting flag.
+        m_statflag |= COUNT_DIRTY;
+    }
 
     Py_DECREF(m_npbuf);
     m_npbuf = NULL;
@@ -808,7 +820,7 @@ FlagtileSurface::flood_fill(const int sx, const int sy, Filler* w)
         if (t != NULL) {
             if (flagtile_flood_fill(t, q, w)) {
                 for(int i=0; i < 4; i++) {
-                    if (! q->is_empty(i)) {
+                    if (! q->is_empty_queue(i)) {
                         int ntx = BorderQueue::adjust_tx(tx, i);
                         int nty = BorderQueue::adjust_ty(ty, i);
                         if(ntx >= 0 && nty >= 0 && ntx < get_width() && nty < get_height()) { 
@@ -1657,12 +1669,16 @@ flagtile_flood_fill(Flagtile *t, BorderQueue *q, Filler *w)
                     break;
                 }
 
-                if (x != x0) { // Test was already done for queued pixels
-                    pix = t->get(level, x, y);
-                    if (!w->match(pix)) {
-                        break;
-                    }
+                // Test was already done for queued pixels ,
+                // However, for initial (border) seed, that test is done against
+                // `pre-filled` pixel. 
+                // Such pixels might be changed by other(previously popped) queue.
+                // So, always do match-check. 
+                pix = t->get(level, x, y);
+                if (!w->match(pix)) {
+                    break;
                 }
+
                 // Operate this pixel, and continue iterating in this direction
                 w->step(t, x, y);
 

lib/pyramidfill.hpp

@@ -68,7 +68,7 @@ class Flagtile
     PyObject *m_npbuf; // Python numpy object, Generated by lock method. 
 
     // Pixel counts. This stores how many pixels per pixel value in this tile.
-    uint16_t m_pixcnt[MAX_PYRAMID][PIXEL_MAX];
+    uint16_t m_pixcnt[MAX_PYRAMID+1][PIXEL_MAX];
 
     // buffer offsets of progress levels.
     static const int m_buf_offsets[MAX_PYRAMID+1];
@@ -92,11 +92,11 @@ class Flagtile
 #ifdef HEAVY_DEBUG
         assert(m_statflag & COUNT_DIRTY);
 #endif
-        memset(m_pixcnt, 0, sizeof(uint16_t) * (PIXEL_MASK+1));
+        memset(m_pixcnt, 0, sizeof(uint16_t) * (MAX_PYRAMID+1) * PIXEL_MAX);
         uint8_t *ptr = m_buf;
         for(int l=0; l < MAX_PYRAMID; l++) {
             for(int i=0; i < PYRAMID_BUF_SIZE(l); i++) {
-                uint8_t pix = *ptr++;
+                uint8_t pix = *(ptr++);
                 m_pixcnt[l][pix & PIXEL_MASK]++;
             }
         }
@@ -128,9 +128,9 @@ class Flagtile
         uint8_t oldpix = *BUF_PTR(level, x, y) & PIXEL_MASK;
         if ((val & PIXEL_MASK) != oldpix) {
             m_pixcnt[level][oldpix]--;
-            m_pixcnt[level][val & PIXEL_MASK]++;
+            m_pixcnt[level][(val & PIXEL_MASK)]++;
         }
-        *BUF_PTR(level, x, y) = val;
+        *BUF_PTR(level, x, y) = val;// Force update, because `val` might contain FLAG_ constants.
     }
 
     void clear_bitwise_flag(const int level, const uint8_t flag)
@@ -198,7 +198,7 @@ class Flagtile
     }
 
     // lock/unlock, to access as numpy buffer from python.
-    PyObject *lock(); 
+    PyObject *lock(const bool readonly); 
     void unlock(PyObject *nparray); 
 
     //// Tile status related
@@ -566,8 +566,7 @@ class BorderQueue
         m_border[dir] ^= ((uint64_t)1 << pos);
     }
 
-
-    inline bool is_empty(const int dir) 
+    inline bool is_empty_queue(const int dir) 
     { 
         if (dir == SPILL_ALL) {
             for(int i=0; i < 4; i++) {
@@ -652,6 +651,7 @@ class BorderQueue
         return gq;
     }
 
+    // XXX for debug
     bool dbg_is_seed(int idx) {
         return m_border[idx] != 0;
     }

lib/pyramidfill.py

@@ -229,7 +229,7 @@ def _trim_tile(ft, tx, ty):
                 sy = max_py + 1 # Start from the next of max_py
             if sx != 0 or sy != 0 or ex != N or ey != N:
        #        print("processing %d, %d - %d,%d to %d, %d" % (tx, ty, sx,sy,ex,ey))
-                npary = tile.lock()
+                npary = tile.lock(True)
                 w = np.reshape(npary[0:N*N], (N,N))
                 w[sy:ey, sx:ex] = _PIXEL.EMPTY
                 tile.unlock(npary)
@@ -309,11 +309,32 @@ def close_fill(targ, src, nodes, targ_pos, level,
                         False
                     )
 
+
     # Build pyramid
     ft.propagate_upward(level)
 
+    # XXX Debug/profiling code
+    if show_flag:
+        _dbg_show_flag(
+            ft, 
+            'contour,filled,outside,area', 
+            title='propagated',
+            level=level
+        )
+    #XXX debug code end
+
     # Deciding outside of filling area
     ft.decide_outside(level)
+    # XXX Debug/profiling code
+    if show_flag:
+        _dbg_show_flag(
+            ft, 
+            'contour,filled,outside,area,workarea', 
+            title='decided',
+            level=level
+        )
+    #XXX debug code end
+
 
     # Then, start progressing tiles.
     for i in range(level, 0, -1):
@@ -843,7 +864,7 @@ def do_fill(targ_pixel, fill_pixel, level):
                #overflows = lib.mypaintlib.flagtile_flood_fill(
                 if lib.mypaintlib.flagtile_flood_fill(flag_tile, eq, w):
                     for i in range(4):
-                        if not eq.is_empty(i):
+                        if not eq.is_empty_queue(i):
                             ntx = lib.mypaintlib.BorderQueue.adjust_tx(tx, i)
                             nty = lib.mypaintlib.BorderQueue.adjust_ty(ty, i)
                             if (ntx <= max_tx and nty <= max_ty
@@ -1000,6 +1021,7 @@ def _dbg_show_flag(ft, method, level=0, title=None):
     # From lib/progfilldefine.hpp
     FLAG_WORK = 0x10
     FLAG_AA = 0x80
+    FLAG_MASK = 0xF0
     npbuf = None
     if title is None:
         title = method
@@ -1043,10 +1065,16 @@ def _dbg_show_flag(ft, method, level=0, title=None):
     def create_npbuf(npbuf, level, pix):
         if npbuf is None:
             npbuf = np.zeros((th*SN, tw*SN, 3), 'uint8')
+
+        if (pix & FLAG_MASK) == 0:
+            color = colors[pix]
+        else:
+            color = colors[pix & FLAG_MASK]
+
         _dbg_render_to_numpy(
             ft,
             npbuf, pix,
-            colors[pix],
+            color,
             level,
             origins=origins
         )
@@ -1060,8 +1088,6 @@ def create_npbuf(npbuf, level, pix):
             npbuf = create_npbuf(npbuf, level, _PIXEL.CONTOUR)
         elif m == 'area':
             npbuf = create_npbuf(npbuf, level, _PIXEL.AREA)
-        elif m == 'close_area':
-            npbuf = create_npbuf(npbuf, 0, _PIXEL.AREA)
         elif m == 'initial_level' or m == 'empty':
             npbuf = create_npbuf(npbuf, level, 0)
         elif m == 'contour':
@@ -1072,6 +1098,8 @@ def create_npbuf(npbuf, level, pix):
             npbuf = create_npbuf(npbuf, level, _PIXEL.OVERWRAP)
         elif m == 'reserve':
             npbuf = create_npbuf(npbuf, level, _PIXEL.RESERVE)
+        elif m == 'workarea':
+            npbuf = create_npbuf(npbuf, level, FLAG_WORK | _PIXEL.AREA)
         elif m == 'level':
             npbuf = create_npbuf(npbuf, level, _PIXEL.AREA)
             npbuf = create_npbuf(npbuf, level, _PIXEL.FILLED)
@@ -1138,7 +1166,7 @@ def _dbg_render_to_numpy(ft, npbuf, pix, color, level, origins=None):
 
     def _render_tile(tile, tx, ty):
         if tile is not None and tile.get_pixel_count(level, pix) > 0:
-            nptile = tile.lock()
+            nptile = tile.lock(True)
             srcview = np.reshape(nptile[l_idx:l_idx+VN*VN], (VN,VN))
             x = tx * VN
             y = ty * VN