Changeset 6725 in vbox for trunk/src/VBox/Frontends

Timestamp:

Feb 1, 2008 4:59:45 PM (17 years ago)

Author:

vboxsync

Message:

A quick implementation sketch of a solution to the SetVisibleRegion vs. paintEvent race.

Location:

trunk/src/VBox/Frontends/VirtualBox

Files:

• include/VBoxFrameBuffer.h (modified) (1 diff)
• src/VBoxFBQuartz2D.cpp (modified) (8 diffs)

trunk/src/VBox/Frontends/VirtualBox/include/VBoxFrameBuffer.h

@@ -509 +509 @@
     ulong mPixelFormat;
     CGImageRef mImage;
+#if 1
     CGRect *mRegionRects;
     ULONG mRegionCount;
-};
-
-#endif
-
-#endif // __VBoxFrameBuffer_h__
+#else
+    typedef struct
+    {
+        /** The size of this structure expressed in rcts entries. */
+        ULONG allocated;
+        /** The number of entries in the rcts array. */
+        ULONG used;
+        /** Variable sized array of the rectangle that makes up the region. */
+        CGRECT rcts[1];
+    } RegionRects;
+    /** The current valid region, all access is by atomic cmpxchg or atomic xchg.
+     *
+     * The protocol for updating and using this has to take into account that
+     * the producer (SetVisibleRegion) and consumer (paintEvent) are running
+     * on different threads. Therefore the producer will create a new RegionRects
+     * structure before atomically replace the existing one. While the consumer
+     * will read the value by atomically replace it by NULL, and then when its
+     * done try restore it by cmpxchg. If the producer has already put a new
+     * region there, it will be discarded (see mRegionUnused).
+     */
+    RegionRects volatile *mRegion;
+    /** For keeping the unused region and thus avoid some RTMemAlloc/RTMemFree calls.
+     * This is operated with atomic cmpxchg and atomic xchg. */
+    RegionRects volatile *mRegionUnused;
+#endif
+};
+
+#endif
+
+#endif // !__VBoxFrameBuffer_h__

trunk/src/VBox/Frontends/VirtualBox/src/VBoxFBQuartz2D.cpp

@@ -41 +41 @@
     mPixelFormat(FramebufferPixelFormat_FOURCC_RGB),
     mImage(NULL),
+#if 1
     mRegionRects(NULL),
     mRegionCount(0)
+#else
+    mRegion (NULL),
+    mRegionUnused (NULL)
+#endif
 {
     Log (("Quartz2D: Creating\n"));
@@ -83 +88 @@
      * ASMAtomic(Cmp)XchgPtr and a struct { cAllocated; cRects; aRects[1]; }
      * *mRegion, *mUnusedRegion; should suffice (and permit you to reuse allocations). */
+#if 1
     RTMemFree (mRegionRects);
     mRegionCount = 0;
     mRegionRects = static_cast <CGRect*> (RTMemAlloc (sizeof (CGRect) * aCount));
+#else
+    RegionRects *rgnRcts = (RegionRects *) ASMAtomicXchgPtr (&mRegionUnused, NULL);
+    if (rgnRcts && rgnRcts->allocated < aCount)
+    {
+        RTMemFree (rgnRcts);
+        rgnRcts = NULL;
+    }
+    if (!rgnRcts)
+    {
+        ULONG allocated = RT_ALIGN_32 (aCount + 1, 32);
+        allocated = RT_MAX (128, allocated);
+        rgnRcts = (RegionRects *) RTMemAlloc (RT_OFFSETOF (RgionRects, rcts[allocated]));
+        if (!rgnRcts)
+            return E_NOMEM;
+        rgnRcts->allocated = allocated;
+    }
+    rgnRcts->used = 0;
+#endif
 
     QRegion reg;
@@ -111 +135 @@
             continue;
 
+#if 1
         mRegionRects[mRegionCount].origin.x = rect.x();
         mRegionRects[mRegionCount].origin.y = rect.y();
@@ -117 +142 @@
 //        printf ("Region rect[%d - %d]: %d %d %d %d\n", mRegionCount, aCount, rect.x(), rect.y(), rect.height(), rect.width());
         ++mRegionCount;
+#else
+        CGRect *cgRct = &rgnRcts->rcts[rgnRcts->used];
+        cgRct->origin.x = rect.x();
+        cgRct->origin.y = rect.y();
+        cgRct->size.width = rect.width();
+        cgRct->size.height = rect.height();
+//        printf ("Region rect[%d - %d]: %d %d %d %d\n", rgnRcts->used, aCount, rect.x(), rect.y(), rect.height(), rect.width());
+        rgnRcts->used++;
+#endif
     }
 //    printf ("..................................\n");
+#if 0
+    void *pvOld = ASMAtomicXchgPtr(&mRegion, rgnRcts);
+    if (    pvOld
+        &&  !ASMAtomicCmpXchgPtr(*mRegionUnused, pvOld, NULL))
+        RTMemFree(pvOld);
+#endif
     QApplication::postEvent (mView, new VBoxSetRegionEvent (reg));
 
@@ -194 +234 @@
         GetPortBounds (GetWindowPort (window), &winRect);
         CGContextClearRect (ctx, CGRectMake (winRect.left, winRect.top, winRect.right - winRect.left, winRect.bottom - winRect.top));
+#if 1
         if (mRegionRects && mRegionCount > 0)
+#else
+        RegionRects *rgnRcts = (RegionRects *) ASMAtomicXchgPtr (&mRegion, NULL);
+        if (rgnRcts)
+#endif
         {
             /* Save state for display fliping */
@@ -204 +249 @@
              * SetVisibleRegion. */
             CGContextBeginPath (ctx);
+#if 1
             CGContextAddRects (ctx, mRegionRects, mRegionCount);
+#else
+            CGContextAddRects (ctx, rgnRects->rcts, rgnRects->used);
+            if (    !ASMAtomicCmpXchgPtr (&mRegion, rgnRects, NULL)
+                &&  !ASMAtomicCmpXchgPtr (&mRegionUnused, rgnRects, NULL))
+                RTMemFree(rgnRects);
+#endif
             /* Restore the context state. Note that the
              * current path isn't destroyed. */
@@ -326 +378 @@
         mBitmapData = NULL;
     }
+#if 1
     if (mRegionRects)
     {
@@ -332 +385 @@
         mRegionCount = 0;
     }
+#else
+    if (mRegion)
+    {
+        RTMemFree (mRegion)
+        mRegion = NULL;
+    }
+    if (mRegionUnused)
+    {
+        RTMemFree (mRegionUnused)
+        mRegionUnused = NULL;
+    }
+#endif
 }