From 4423855cbd5411b64deba32e8b58af8b9c708ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Wed, 13 Sep 2006 20:02:00 +0000 Subject: [PATCH] This is the kind of bug you can search for hours. At first, I suspected a concurency problem in the DrawingEngine, so there is some debugging stuff added, as well as some unnecessary locking removed there. The problem was in Painter though, in that certain functions adjusted clipping at the "rasterizer level", while some other functions didn't care about that. Now the clipping is consistently set at the rasterizer level (rough estimate to avoid scanline generation outside real clipping region bounds). There are a number of bugs fixed by this, I'm going to find out later, what their ticket numbers are... Mouse preflet draws the mouse now, Backgrounds preflet draws the screen reliably... probably more, anything to do with bitmaps, round rects and possibly ellipses. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@18828 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/servers/app/drawing/DrawingEngine.cpp | 194 ++++++++++++-------- src/servers/app/drawing/DrawingEngine.h | 1 + src/servers/app/drawing/Painter/Painter.cpp | 11 +- 3 files changed, 120 insertions(+), 86 deletions(-) diff --git a/src/servers/app/drawing/DrawingEngine.cpp b/src/servers/app/drawing/DrawingEngine.cpp index 4f3cb4f73f..7f61b8cef1 100644 --- a/src/servers/app/drawing/DrawingEngine.cpp +++ b/src/servers/app/drawing/DrawingEngine.cpp @@ -23,6 +23,9 @@ #include "drawing_support.h" +#define CRASH_IF_NOT_LOCKED +//#define CRASH_IF_NOT_LOCKED if (!IsLocked()) debugger("not locked!"); + // make_rect_valid static inline void make_rect_valid(BRect& rect) @@ -138,6 +141,8 @@ DrawingEngine::SetHWInterface(HWInterface* interface) void DrawingEngine::ConstrainClippingRegion(const BRegion* region) { + CRASH_IF_NOT_LOCKED + fPainter->ConstrainClipping(region); } @@ -145,6 +150,8 @@ DrawingEngine::ConstrainClippingRegion(const BRegion* region) void DrawingEngine::SuspendAutoSync() { + CRASH_IF_NOT_LOCKED + fSuspendSyncLevel++; } @@ -152,6 +159,8 @@ DrawingEngine::SuspendAutoSync() void DrawingEngine::Sync() { + CRASH_IF_NOT_LOCKED + fSuspendSyncLevel--; if (fSuspendSyncLevel == 0) fGraphicsCard->Sync(); @@ -426,19 +435,17 @@ DrawingEngine::DrawBitmap(ServerBitmap *bitmap, const BRect &source, const BRect &dest, const DrawState *d) { - if (Lock()) { - BRect clipped = fPainter->ClipRect(dest); - if (clipped.IsValid()) { - fGraphicsCard->HideSoftwareCursor(clipped); - - fPainter->SetDrawState(d); - fPainter->DrawBitmap(bitmap, source, dest); - - fGraphicsCard->Invalidate(clipped); - fGraphicsCard->ShowSoftwareCursor(); - } + CRASH_IF_NOT_LOCKED - Unlock(); + BRect clipped = fPainter->ClipRect(dest); + if (clipped.IsValid()) { + fGraphicsCard->HideSoftwareCursor(clipped); + + fPainter->SetDrawState(d); + fPainter->DrawBitmap(bitmap, source, dest); + + fGraphicsCard->Invalidate(clipped); + fGraphicsCard->ShowSoftwareCursor(); } } @@ -448,32 +455,30 @@ DrawingEngine::DrawArc(BRect r, const float &angle, const float &span, const DrawState *d, bool filled) { - if (Lock()) { - make_rect_valid(r); - BRect clipped(r); - if (!filled) - extend_by_stroke_width(clipped, d); - clipped = fPainter->ClipRect(r); - if (clipped.IsValid()) { - fGraphicsCard->HideSoftwareCursor(clipped); - - fPainter->SetDrawState(d); - - float xRadius = r.Width() / 2.0; - float yRadius = r.Height() / 2.0; - BPoint center(r.left + xRadius, - r.top + yRadius); + CRASH_IF_NOT_LOCKED - if (filled) - fPainter->FillArc(center, xRadius, yRadius, angle, span); - else - fPainter->StrokeArc(center, xRadius, yRadius, angle, span); - - fGraphicsCard->Invalidate(clipped); - fGraphicsCard->ShowSoftwareCursor(); - } + make_rect_valid(r); + BRect clipped(r); + if (!filled) + extend_by_stroke_width(clipped, d); + clipped = fPainter->ClipRect(r); + if (clipped.IsValid()) { + fGraphicsCard->HideSoftwareCursor(clipped); - Unlock(); + fPainter->SetDrawState(d); + + float xRadius = r.Width() / 2.0; + float yRadius = r.Height() / 2.0; + BPoint center(r.left + xRadius, + r.top + yRadius); + + if (filled) + fPainter->FillArc(center, xRadius, yRadius, angle, span); + else + fPainter->StrokeArc(center, xRadius, yRadius, angle, span); + + fGraphicsCard->Invalidate(clipped); + fGraphicsCard->ShowSoftwareCursor(); } } @@ -481,48 +486,44 @@ DrawingEngine::DrawArc(BRect r, const float &angle, void DrawingEngine::DrawBezier(BPoint *pts, const DrawState *d, bool filled) { - if (Lock()) { - // TODO: figure out bounds and hide cursor depending on that - fGraphicsCard->HideSoftwareCursor(); + CRASH_IF_NOT_LOCKED - fPainter->SetDrawState(d); - BRect touched = fPainter->DrawBezier(pts, filled); + // TODO: figure out bounds and hide cursor depending on that + fGraphicsCard->HideSoftwareCursor(); - fGraphicsCard->Invalidate(touched); - fGraphicsCard->ShowSoftwareCursor(); + fPainter->SetDrawState(d); + BRect touched = fPainter->DrawBezier(pts, filled); - Unlock(); - } + fGraphicsCard->Invalidate(touched); + fGraphicsCard->ShowSoftwareCursor(); } // DrawEllipse void DrawingEngine::DrawEllipse(BRect r, const DrawState *d, bool filled) { - if (Lock()) { - make_rect_valid(r); - BRect clipped = r; - fPainter->AlignEllipseRect(&clipped, filled); - if (!filled) - extend_by_stroke_width(clipped, d); + CRASH_IF_NOT_LOCKED - clipped.left = floorf(clipped.left); - clipped.top = floorf(clipped.top); - clipped.right = ceilf(clipped.right); - clipped.bottom = ceilf(clipped.bottom); + make_rect_valid(r); + BRect clipped = r; + fPainter->AlignEllipseRect(&clipped, filled); + if (!filled) + extend_by_stroke_width(clipped, d); - clipped = fPainter->ClipRect(clipped); - if (clipped.IsValid()) { - fGraphicsCard->HideSoftwareCursor(clipped); - - fPainter->SetDrawState(d); - fPainter->DrawEllipse(r, filled); - - fGraphicsCard->Invalidate(clipped); - fGraphicsCard->ShowSoftwareCursor(); - } + clipped.left = floorf(clipped.left); + clipped.top = floorf(clipped.top); + clipped.right = ceilf(clipped.right); + clipped.bottom = ceilf(clipped.bottom); - Unlock(); + clipped = fPainter->ClipRect(clipped); + if (clipped.IsValid()) { + fGraphicsCard->HideSoftwareCursor(clipped); + + fPainter->SetDrawState(d); + fPainter->DrawEllipse(r, filled); + + fGraphicsCard->Invalidate(clipped); + fGraphicsCard->ShowSoftwareCursor(); } } @@ -532,22 +533,20 @@ DrawingEngine::DrawPolygon(BPoint* ptlist, int32 numpts, BRect bounds, const DrawState* d, bool filled, bool closed) { - if (Lock()) { - make_rect_valid(bounds); - if (!filled) - extend_by_stroke_width(bounds, d); - bounds = fPainter->ClipRect(bounds); - if (bounds.IsValid()) { - fGraphicsCard->HideSoftwareCursor(bounds); - - fPainter->SetDrawState(d); - fPainter->DrawPolygon(ptlist, numpts, filled, closed); - - fGraphicsCard->Invalidate(bounds); - fGraphicsCard->ShowSoftwareCursor(); - } + CRASH_IF_NOT_LOCKED - Unlock(); + make_rect_valid(bounds); + if (!filled) + extend_by_stroke_width(bounds, d); + bounds = fPainter->ClipRect(bounds); + if (bounds.IsValid()) { + fGraphicsCard->HideSoftwareCursor(bounds); + + fPainter->SetDrawState(d); + fPainter->DrawPolygon(ptlist, numpts, filled, closed); + + fGraphicsCard->Invalidate(bounds); + fGraphicsCard->ShowSoftwareCursor(); } } @@ -566,6 +565,8 @@ DrawingEngine::StrokePoint(const BPoint& pt, const RGBColor &color) void DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, const RGBColor &color) { + CRASH_IF_NOT_LOCKED + if (Lock()) { BRect touched(start, end); make_rect_valid(touched); @@ -589,6 +590,8 @@ DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, const RGBColor void DrawingEngine::StrokeRect(BRect r, const RGBColor &color) { + CRASH_IF_NOT_LOCKED + if (Lock()) { make_rect_valid(r); BRect clipped = fPainter->ClipRect(r); @@ -609,6 +612,8 @@ DrawingEngine::StrokeRect(BRect r, const RGBColor &color) void DrawingEngine::FillRect(BRect r, const RGBColor& color) { + CRASH_IF_NOT_LOCKED + // NOTE: Write locking because we might use HW acceleration. // This needs to be investigated, I'm doing this because of // gut feeling. @@ -643,6 +648,8 @@ DrawingEngine::FillRect(BRect r, const RGBColor& color) void DrawingEngine::FillRegion(BRegion& r, const RGBColor& color) { + CRASH_IF_NOT_LOCKED + // NOTE: Write locking because we might use HW acceleration. // This needs to be investigated, I'm doing this because of // gut feeling. @@ -682,6 +689,8 @@ DrawingEngine::FillRegion(BRegion& r, const RGBColor& color) void DrawingEngine::StrokeRect(BRect r, const DrawState *d) { + CRASH_IF_NOT_LOCKED + // support invalid rects make_rect_valid(r); BRect clipped(r); @@ -703,6 +712,8 @@ DrawingEngine::StrokeRect(BRect r, const DrawState *d) void DrawingEngine::FillRect(BRect r, const DrawState *d) { + CRASH_IF_NOT_LOCKED + // NOTE: Write locking because we might use HW acceleration. // This needs to be investigated, I'm doing this because of // gut feeling. @@ -756,6 +767,8 @@ DrawingEngine::FillRect(BRect r, const DrawState *d) void DrawingEngine::FillRegion(BRegion& r, const DrawState *d) { + CRASH_IF_NOT_LOCKED + // NOTE: Write locking because we might use HW acceleration. // This needs to be investigated, I'm doing this because of // gut feeling. @@ -811,6 +824,8 @@ void DrawingEngine::DrawRoundRect(BRect r, float xrad, float yrad, const DrawState* d, bool filled) { + CRASH_IF_NOT_LOCKED + // NOTE: the stroke does not extend past "r" in R5, // though I consider this unexpected behaviour. make_rect_valid(r); @@ -839,6 +854,8 @@ DrawingEngine::DrawShape(const BRect& bounds, int32 opCount, const uint32* opList, int32 ptCount, const BPoint* ptList, const DrawState* d, bool filled) { + CRASH_IF_NOT_LOCKED + // NOTE: hides cursor regardless of if and where // shape is drawn on screen, TODO: optimize fGraphicsCard->HideSoftwareCursor(); @@ -857,6 +874,8 @@ void DrawingEngine::DrawTriangle(BPoint* pts, const BRect& bounds, const DrawState* d, bool filled) { + CRASH_IF_NOT_LOCKED + BRect clipped(bounds); if (!filled) extend_by_stroke_width(clipped, d); @@ -879,6 +898,8 @@ DrawingEngine::DrawTriangle(BPoint* pts, const BRect& bounds, void DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, DrawState* context) { + CRASH_IF_NOT_LOCKED + BRect touched(start, end); make_rect_valid(touched); extend_by_stroke_width(touched, context); @@ -899,6 +920,8 @@ void DrawingEngine::StrokeLineArray(int32 numLines, const LineArrayData *linedata, const DrawState *d) { + CRASH_IF_NOT_LOCKED + if (!d || !linedata || numLines <= 0) return; @@ -953,6 +976,8 @@ DrawingEngine::DrawString(const char* string, int32 length, const BPoint& pt, DrawState* d, escapement_delta* delta) { + CRASH_IF_NOT_LOCKED + // TODO: use delta FontLocker locker(d); @@ -1057,6 +1082,13 @@ DrawingEngine::Unlock() fGraphicsCard->WriteUnlock(); } +// IsLocked +bool +DrawingEngine::IsLocked() +{ + return fGraphicsCard->IsWriteLocked(); +} + // WriteLock bool DrawingEngine::WriteLock() diff --git a/src/servers/app/drawing/DrawingEngine.h b/src/servers/app/drawing/DrawingEngine.h index 9dfec9b6d6..6c11636838 100644 --- a/src/servers/app/drawing/DrawingEngine.h +++ b/src/servers/app/drawing/DrawingEngine.h @@ -47,6 +47,7 @@ public: // locking bool Lock(); void Unlock(); + bool IsLocked(); bool WriteLock(); void WriteUnlock(); diff --git a/src/servers/app/drawing/Painter/Painter.cpp b/src/servers/app/drawing/Painter/Painter.cpp index 225445ed5a..a5275e8991 100644 --- a/src/servers/app/drawing/Painter/Painter.cpp +++ b/src/servers/app/drawing/Painter/Painter.cpp @@ -234,6 +234,11 @@ Painter::ConstrainClipping(const BRegion* region) { *fClippingRegion = *region; fValidClipping = fClippingRegion->Frame().IsValid(); + + if (fValidClipping) { + clipping_rect cb = fClippingRegion->FrameInt(); + fRasterizer->clip_box(cb.left, cb.top, cb.right + 1, cb.bottom + 1); + } // TODO: would be nice if we didn't need to copy a region // for *each* drawing command... //fBaseRenderer->set_clipping_region(const_cast(region)); @@ -1570,7 +1575,6 @@ Painter::_DrawBitmapGeneric32(agg::rendering_buffer& srcBuffer, span_gen_type spanGenerator(source, interpolator); - // clip to the current clipping region's frame viewRect = viewRect & fClippingRegion->Frame(); // convert to pixel coords (versus pixel indices) @@ -1586,6 +1590,7 @@ Painter::_DrawBitmapGeneric32(agg::rendering_buffer& srcBuffer, fPath.close_polygon(); agg::conv_transform transformedPath(fPath, srcMatrix); + fRasterizer->reset(); fRasterizer->add_path(transformedPath); // render the path with the bitmap as scanline fill @@ -1719,8 +1724,6 @@ Painter::_StrokePath(VertexSource& path) const stroke.miter_limit(fMiterLimit); fRasterizer->reset(); - clipping_rect cb = fClippingRegion->FrameInt(); - fRasterizer->clip_box(cb.left, cb.top, cb.right + 1, cb.bottom + 1); fRasterizer->add_path(stroke); agg::render_scanlines(*fRasterizer, *fPackedScanline, *fRenderer); @@ -1744,8 +1747,6 @@ BRect Painter::_FillPath(VertexSource& path) const { fRasterizer->reset(); - clipping_rect cb = fClippingRegion->FrameInt(); - fRasterizer->clip_box(cb.left, cb.top, cb.right + 1, cb.bottom + 1); fRasterizer->add_path(path); agg::render_scanlines(*fRasterizer, *fPackedScanline, *fRenderer);