From 2469a6f4272b43fb04e11290808216f6a7610599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Thu, 23 Oct 2008 17:15:39 +0000 Subject: [PATCH] * Rewrote the UpdateQueue class. It actually works now and would perform screen updates during the vertical refresh, but it causes flickering again since there is no guarantee that screen regions will stay clean from the time that they were scheduled with the UpdateQueue until the UpdateQueue thread transfers them. Therefor it is still disabled. * Refactored a bit the distinction between Invalidate() and CopyToFront(). Invalidate() used to be virtual, but now CopyToFront() is. This was mainly needed for the app_server test environment, because the host window needs to call Invalidate() when the front buffer bitmap is clean. When the UpdateQueue is used, this needs to be CopyToFront(). Now the separation is cleaner in combination with the UpdateQueue. * Fixed a problem in HWInterface::CopyToFront(): When separating the region outside the cursor and the region with the cursor during a transfer, it needs to hold the fFloatingOverlay lock to make sure the cursor is not moved in the meantime. This fixes graphics glitches with remnants of the cursor staying on screen. These could very rarely be observed, but much more often with the accelerated double-buffer mode. * Enabled the accelerated double buffered mode, since it works now very well. I was able to test it with the nVidea driver on an nVideo 7300. It works by allocating a frame buffer twice the height of the configured screen mode. Then all drawing goes into the offscreen portion, including accelerated driver functions. AccelerantHWInterface::_CopyToFront() then uses acceleration to blit the clean regions in the offscreen portion of the frame buffer into the visible part. Please tell me if there are problems, for example when if there is too few video memory, or if a driver does not handle it correctly. To disable it, see src/servers/app/drawing/AccelerantHWInterface.cpp line 511. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28301 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- .../app/drawing/AccelerantHWInterface.cpp | 11 +- .../app/drawing/AccelerantHWInterface.h | 2 +- src/servers/app/drawing/BitmapHWInterface.cpp | 2 +- src/servers/app/drawing/HWInterface.cpp | 70 +++++-- src/servers/app/drawing/HWInterface.h | 14 +- src/servers/app/drawing/UpdateQueue.cpp | 193 +++++++++++------- src/servers/app/drawing/UpdateQueue.h | 25 ++- src/servers/app/drawing/ViewHWInterface.cpp | 11 +- src/servers/app/drawing/ViewHWInterface.h | 2 +- 9 files changed, 209 insertions(+), 121 deletions(-) diff --git a/src/servers/app/drawing/AccelerantHWInterface.cpp b/src/servers/app/drawing/AccelerantHWInterface.cpp index 5bde8e2287..0051add0b8 100644 --- a/src/servers/app/drawing/AccelerantHWInterface.cpp +++ b/src/servers/app/drawing/AccelerantHWInterface.cpp @@ -508,7 +508,7 @@ AccelerantHWInterface::SetMode(const display_mode& mode) bool tryOffscreenBackBuffer = false; fOffscreenBackBuffer = false; -#if 0 +#if 1 if (fVGADevice < 0 && (color_space)newMode.space == B_RGB32) { // we should have an accelerated graphics driver, try // to allocate a frame buffer large enough to contain @@ -647,6 +647,11 @@ AccelerantHWInterface::SetMode(const display_mode& mode) // clear out backbuffer, alpha is 255 this way memset(fBackBuffer->Bits(), 255, fBackBuffer->BitsLength()); } +#if 0 +// NOTE: Currently disabled, because it make the double buffered mode flicker +// again. See HWInterface::Invalidate() for more information. + SetAsyncDoubleBuffered(doubleBuffered); +#endif } // update color palette configuration if necessary @@ -1342,7 +1347,7 @@ AccelerantHWInterface::IsDoubleBuffered() const void -AccelerantHWInterface::CopyBackToFront(/*const*/ BRegion& region) +AccelerantHWInterface::_CopyBackToFront(/*const*/ BRegion& region) { if (fOffscreenBackBuffer) { int32 xOffset = 0; @@ -1361,7 +1366,7 @@ AccelerantHWInterface::CopyBackToFront(/*const*/ BRegion& region) return; } - return HWInterface::CopyBackToFront(region); + return HWInterface::_CopyBackToFront(region); } diff --git a/src/servers/app/drawing/AccelerantHWInterface.h b/src/servers/app/drawing/AccelerantHWInterface.h index 9f14127189..a8e2328130 100644 --- a/src/servers/app/drawing/AccelerantHWInterface.h +++ b/src/servers/app/drawing/AccelerantHWInterface.h @@ -95,7 +95,7 @@ public: virtual bool IsDoubleBuffered() const; protected: - virtual void CopyBackToFront(/*const*/ BRegion& region); + virtual void _CopyBackToFront(/*const*/ BRegion& region); virtual void _DrawCursor(IntRect area) const; diff --git a/src/servers/app/drawing/BitmapHWInterface.cpp b/src/servers/app/drawing/BitmapHWInterface.cpp index c85ce06612..5c251bf7e9 100644 --- a/src/servers/app/drawing/BitmapHWInterface.cpp +++ b/src/servers/app/drawing/BitmapHWInterface.cpp @@ -23,7 +23,7 @@ using std::nothrow; BitmapHWInterface::BitmapHWInterface(ServerBitmap* bitmap) - : HWInterface(), + : HWInterface(false, false), fBackBuffer(NULL), fFrontBuffer(new(nothrow) BitmapBuffer(bitmap)) { diff --git a/src/servers/app/drawing/HWInterface.cpp b/src/servers/app/drawing/HWInterface.cpp index 1a6e5946f1..47e83dbe20 100644 --- a/src/servers/app/drawing/HWInterface.cpp +++ b/src/servers/app/drawing/HWInterface.cpp @@ -9,17 +9,21 @@ #include "HWInterface.h" +#include +#include +#include +#include + +#include + #include "drawing_support.h" #include "RenderingBuffer.h" #include "SystemPalette.h" #include "UpdateQueue.h" -#include -#include -#include -#include +using std::nothrow; HWInterfaceListener::HWInterfaceListener() @@ -35,7 +39,7 @@ HWInterfaceListener::~HWInterfaceListener() // #pragma mark - HWInterface -HWInterface::HWInterface(bool doubleBuffered) +HWInterface::HWInterface(bool doubleBuffered, bool enableUpdateQueue) : MultiLocker("hw interface lock"), fCursorAreaBackup(NULL), fFloatingOverlaysLock("floating overlays lock"), @@ -48,22 +52,22 @@ HWInterface::HWInterface(bool doubleBuffered) fCursorLocation(0, 0), fDoubleBuffered(doubleBuffered), fVGADevice(-1), -// fUpdateExecutor(new UpdateQueue(this)) fUpdateExecutor(NULL), fListeners(20) { + SetAsyncDoubleBuffered(doubleBuffered && enableUpdateQueue); } // destructor HWInterface::~HWInterface() { + SetAsyncDoubleBuffered(false); + delete fCursorAreaBackup; // The standard cursor doesn't belong us - the drag bitmap might if (fCursor != fCursorAndDragBitmap) delete fCursorAndDragBitmap; - - delete fUpdateExecutor; } // Initialize @@ -286,6 +290,24 @@ HWInterface::DrawingBuffer() const } +void +HWInterface::SetAsyncDoubleBuffered(bool doubleBuffered) +{ + if (doubleBuffered) { + if (fUpdateExecutor != NULL) + return; + fUpdateExecutor = new (nothrow) UpdateQueue(this); + AddListener(fUpdateExecutor); + } else { + if (fUpdateExecutor == NULL) + return; + RemoveListener(fUpdateExecutor); + delete fUpdateExecutor; + fUpdateExecutor = NULL; + } +} + + bool HWInterface::IsDoubleBuffered() const { @@ -298,20 +320,21 @@ status_t HWInterface::Invalidate(const BRect& frame) { if (IsDoubleBuffered()) { - return CopyBackToFront(frame); - -// TODO: the remaining problem is the immediate wake up of the -// thread carrying out the updates, when I enable it, there -// seems to be a deadlock, but I didn't figure it out yet. -// Maybe the same bug is there without the wakeup, only, triggered -// less often.... scarry, huh? -/* if (frame.IsValid()) { +#if 0 +// NOTE: The UpdateQueue works perfectly fine, but it screws the +// flicker-free-ness of the double buffered rendering. The problem being the +// asynchronous nature. The UpdateQueue will transfer regions of the screen +// which have been clean at the time we are in this function, but which have +// been damaged meanwhile by drawing into them again. All in all, the +// UpdateQueue is good for reducing the number of times that the transfer +// is performed, and makes it happen during refresh only, but until there +// is a smarter way to synchronize this all better, I've disabled it. + if (fUpdateExecutor != NULL) { fUpdateExecutor->AddRect(frame); return B_OK; } - return B_BAD_VALUE;*/ - } else { -// _DrawCursor(frame); +#endif + return CopyBackToFront(frame); } return B_OK; } @@ -336,14 +359,19 @@ HWInterface::CopyBackToFront(const BRect& frame) // make sure we don't copy out of bounds area = bufferClip & area; + bool cursorLocked = fFloatingOverlaysLock.Lock(); + BRegion region((BRect)area); if (IsDoubleBuffered()) region.Exclude((clipping_rect)_CursorFrame()); - CopyBackToFront(region); + _CopyBackToFront(region); _DrawCursor(area); + if (cursorLocked) + fFloatingOverlaysLock.Unlock(); + return B_OK; } return B_BAD_VALUE; @@ -351,7 +379,7 @@ HWInterface::CopyBackToFront(const BRect& frame) void -HWInterface::CopyBackToFront(/*const*/ BRegion& region) +HWInterface::_CopyBackToFront(/*const*/ BRegion& region) { RenderingBuffer* backBuffer = BackBuffer(); diff --git a/src/servers/app/drawing/HWInterface.h b/src/servers/app/drawing/HWInterface.h index 816aabcabf..1e8589c676 100644 --- a/src/servers/app/drawing/HWInterface.h +++ b/src/servers/app/drawing/HWInterface.h @@ -45,7 +45,8 @@ class HWInterfaceListener { class HWInterface : protected MultiLocker { public: - HWInterface(bool doubleBuffered = false); + HWInterface(bool doubleBuffered = false, + bool enableUpdateQueue = true); virtual ~HWInterface(); // locking @@ -138,16 +139,17 @@ class HWInterface : protected MultiLocker { RenderingBuffer* DrawingBuffer() const; virtual RenderingBuffer* FrontBuffer() const = 0; virtual RenderingBuffer* BackBuffer() const = 0; + void SetAsyncDoubleBuffered(bool doubleBuffered); virtual bool IsDoubleBuffered() const; - // Invalidate is planned to be used for scheduling an area for updating - // you need to WriteLock! - virtual status_t Invalidate(const BRect& frame); + // Invalidate is used for scheduling an area for updating + status_t Invalidate(const BRect& frame); // while as CopyBackToFront() actually performs the operation - status_t CopyBackToFront(const BRect& frame); + // either directly or asynchronously by the UpdateQueue thread + virtual status_t CopyBackToFront(const BRect& frame); protected: - virtual void CopyBackToFront(/*const*/ BRegion& region); + virtual void _CopyBackToFront(/*const*/ BRegion& region); public: // TODO: Just a quick and primitive way to get single buffered mode working. diff --git a/src/servers/app/drawing/UpdateQueue.cpp b/src/servers/app/drawing/UpdateQueue.cpp index 5ff5fc4934..d16a3e3ff6 100644 --- a/src/servers/app/drawing/UpdateQueue.cpp +++ b/src/servers/app/drawing/UpdateQueue.cpp @@ -1,73 +1,119 @@ -// UpdateQueue.cpp +/* + * Copyright 2005-2008 Stephan Aßmus . All rights reserved. + * Distributed under the terms of the MIT License. + */ +#include "UpdateQueue.h" #include #include #include -#include "HWInterface.h" -#include "UpdateQueue.h" +//#define TRACE_UPDATE_QUEUE +#ifdef TRACE_UPDATE_QUEUE +# include +# include + + static int32 sFunctionDepth = -1; +# define CALLED(x...) FunctionTracer _ft("UpdateQueue", __FUNCTION__, \ + sFunctionDepth) +# define TRACE(x...) { BString _to; \ + _to.Append(' ', (sFunctionDepth + 1) * 2); \ + printf("%s", _to.String()); printf(x); } +#else +# define CALLED(x...) +# define TRACE(x...) +#endif + // constructor UpdateQueue::UpdateQueue(HWInterface* interface) - : BLocker("AppServer_UpdateQueue"), + : + BLocker("AppServer_UpdateQueue"), + fQuitting(false), fInterface(interface), fUpdateRegion(), - fUpdateExecutor(-1), - fThreadControl(-1), - fStatus(B_ERROR) + fUpdateExecutor(B_BAD_THREAD_ID), + fRetraceSem(B_BAD_SEM_ID), + fRefreshDuration(1000000 / 60) { - fThreadControl = create_sem(0, "update queue control"); - if (fThreadControl >= B_OK) - fStatus = B_OK; - else - fStatus = fThreadControl; - if (fStatus == B_OK) { - fUpdateExecutor = spawn_thread(_execute_updates_, "update queue runner", - B_NORMAL_PRIORITY, this); - if (fUpdateExecutor >= B_OK) { - fStatus = B_OK; - resume_thread(fUpdateExecutor); - } else - fStatus = fUpdateExecutor; - } + CALLED(); + TRACE("this: %p\n", this); + TRACE("fInterface: %p\n", fInterface); } // destructor UpdateQueue::~UpdateQueue() { - if (delete_sem(fThreadControl) == B_OK) - wait_for_thread(fUpdateExecutor, &fUpdateExecutor); + CALLED(); + + Shutdown(); } -// InitCheck -status_t -UpdateQueue::InitCheck() +// FrameBufferChanged +void +UpdateQueue::FrameBufferChanged() { - return fStatus; + CALLED(); + + Init(); +} + +// Init +status_t +UpdateQueue::Init() +{ + CALLED(); + + Shutdown(); + + fRetraceSem = fInterface->RetraceSemaphore(); +// fRefreshDuration = fInterface->... + + TRACE("fRetraceSem: %ld, fRefreshDuration: %lld\n", + fRetraceSem, fRefreshDuration); + + fQuitting = false; + fUpdateExecutor = spawn_thread(_ExecuteUpdatesEntry, "update queue runner", + B_REAL_TIME_PRIORITY, this); + if (fUpdateExecutor < B_OK) + return fUpdateExecutor; + + return resume_thread(fUpdateExecutor); +} + +// Shutdown +void +UpdateQueue::Shutdown() +{ + CALLED(); + + if (fUpdateExecutor < B_OK) + return; + fQuitting = true; + status_t exitValue; + wait_for_thread(fUpdateExecutor, &exitValue); + fUpdateExecutor = B_BAD_THREAD_ID; } // AddRect void UpdateQueue::AddRect(const BRect& rect) { -// Lock(); -//printf("UpdateQueue::AddRect()\n"); - // NOTE: The access to fUpdateRegion - // is protected by the HWInterface lock. - // When trying to access the fUpdateRegion, - // our thread will first try to lock the - // HWInterface, while on the other hand - // HWInterface will always be locked when - // it calls AddRect(). - fUpdateRegion.Include(rect); -// _Reschedule(); -// Unlock(); + if (!rect.IsValid()) + return; + + CALLED(); + + if (Lock()) { + fUpdateRegion.Include(rect); + Unlock(); + } } -// _execute_updates_ +// _ExecuteUpdatesEntry int32 -UpdateQueue::_execute_updates_(void* cookie) +UpdateQueue::_ExecuteUpdatesEntry(void* cookie) { UpdateQueue *gc = (UpdateQueue*)cookie; return gc->_ExecuteUpdates(); @@ -77,51 +123,50 @@ UpdateQueue::_execute_updates_(void* cookie) int32 UpdateQueue::_ExecuteUpdates() { - bool running = true; - while (running) { - status_t err = acquire_sem_etc(fThreadControl, 1, B_RELATIVE_TIMEOUT, - 20000); + while (!fQuitting) { + status_t err; + if (fRetraceSem >= 0) { + bigtime_t timeout = system_time() + fRefreshDuration * 2; +// TRACE("acquire_sem_etc(%lld)\n", timeout); + do { + err = acquire_sem_etc(fRetraceSem, 1, + B_ABSOLUTE_TIMEOUT | B_CAN_INTERRUPT, timeout); + } while (err == B_INTERRUPTED && !fQuitting); + } else { + bigtime_t timeout = system_time() + fRefreshDuration; +// TRACE("snooze_until(%lld)\n", timeout); + do { + err = snooze_until(timeout, B_SYSTEM_TIMEBASE); + } while (err == B_INTERRUPTED && !fQuitting); + } + if (fQuitting) + return B_OK; switch (err) { case B_OK: case B_TIMED_OUT: // execute updates if (fInterface->LockParallelAccess()) { - int32 count = fUpdateRegion.CountRects(); - if (count > 0) { - for (int32 i = 0; i < count; i++) { - fInterface->CopyBackToFront(fUpdateRegion.RectAt(i)); + if (Lock()) { + int32 count = fUpdateRegion.CountRects(); + if (count > 0) { + TRACE("CopyBackToFront() - rects: %ld\n", count); + // NOTE: not using the BRegion version, since that + // doesn't take care of leaving out and compositing + // the cursor. + for (int32 i = 0; i < count; i++) + fInterface->CopyBackToFront( + fUpdateRegion.RectAt(i)); + fUpdateRegion.MakeEmpty(); } - fUpdateRegion.MakeEmpty(); + Unlock(); } fInterface->UnlockParallelAccess(); } break; - case B_BAD_SEM_ID: - running = false; - break; default: -printf("other error: %s\n", strerror(err)); -//running = false; - snooze(20000); - break; + return err; } } - return 0; -} - -// _Reschedule -// -// PRE: The object must be locked. -void -UpdateQueue::_Reschedule() -{ - // TODO: _Reschedule() is supposed to cause the - // immediate wake up of the update thread, but - // the HWInterface is still locked when we get here. - // Somehow this causes a deadlock, but I don't - // see why yet... - if (fStatus == B_OK) { - release_sem(fThreadControl); - } + return B_OK; } diff --git a/src/servers/app/drawing/UpdateQueue.h b/src/servers/app/drawing/UpdateQueue.h index 074c16ff0c..f8c5614cec 100644 --- a/src/servers/app/drawing/UpdateQueue.h +++ b/src/servers/app/drawing/UpdateQueue.h @@ -1,5 +1,7 @@ -// UpdateQueue.h - +/* + * Copyright 2005-2008 Stephan Aßmus . All rights reserved. + * Distributed under the terms of the MIT License. + */ #ifndef UPDATE_QUEUE_H #define UPDATE_QUEUE_H @@ -8,29 +10,32 @@ #include #include -class HWInterface; +#include "HWInterface.h" -class UpdateQueue : public BLocker { + +class UpdateQueue : public BLocker, public HWInterfaceListener { public: UpdateQueue(HWInterface* interface); virtual ~UpdateQueue(); - status_t InitCheck(); + virtual void FrameBufferChanged(); + + status_t Init(); + void Shutdown(); void AddRect(const BRect& rect); private: - static int32 _execute_updates_(void *cookie); + static int32 _ExecuteUpdatesEntry(void *cookie); int32 _ExecuteUpdates(); - void _Reschedule(); - + volatile bool fQuitting; HWInterface* fInterface; BRegion fUpdateRegion; thread_id fUpdateExecutor; - sem_id fThreadControl; - status_t fStatus; + sem_id fRetraceSem; + bigtime_t fRefreshDuration; }; #endif // UPDATE_QUEUE_H diff --git a/src/servers/app/drawing/ViewHWInterface.cpp b/src/servers/app/drawing/ViewHWInterface.cpp index 55abfa9382..ad2c139be3 100644 --- a/src/servers/app/drawing/ViewHWInterface.cpp +++ b/src/servers/app/drawing/ViewHWInterface.cpp @@ -385,8 +385,11 @@ CardWindow::Invalidate(const BRect& frame) // #pragma mark - +static const bool kDefaultDoubleBuffered = true; + + ViewHWInterface::ViewHWInterface() - : HWInterface(), + : HWInterface(kDefaultDoubleBuffered), fBackBuffer(NULL), fFrontBuffer(NULL), fWindow(NULL) @@ -755,11 +758,11 @@ ViewHWInterface::IsDoubleBuffered() const return HWInterface::IsDoubleBuffered(); } -// Invalidate +// CopyBackToFront status_t -ViewHWInterface::Invalidate(const BRect& frame) +ViewHWInterface::CopyBackToFront(const BRect& frame) { - status_t ret = HWInterface::Invalidate(frame); + status_t ret = HWInterface::CopyBackToFront(frame); if (ret >= B_OK && fWindow) fWindow->Invalidate(frame); diff --git a/src/servers/app/drawing/ViewHWInterface.h b/src/servers/app/drawing/ViewHWInterface.h index 881ecab509..a35cff3b1c 100644 --- a/src/servers/app/drawing/ViewHWInterface.h +++ b/src/servers/app/drawing/ViewHWInterface.h @@ -53,7 +53,7 @@ class ViewHWInterface : public HWInterface { virtual RenderingBuffer* BackBuffer() const; virtual bool IsDoubleBuffered() const; - virtual status_t Invalidate(const BRect& frame); + virtual status_t CopyBackToFront(const BRect& frame); private: BBitmapBuffer* fBackBuffer;