From 79ef179c52ff23c1338a34a95452114d51bffeda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 18 Mar 2008 00:04:12 +0000 Subject: [PATCH] A test app revealed some bugs with regards to client provided clipping regions: * It is necessary to store the local origin and scale of a view state, at least for the clipping region and especially because the scale affects the clipping region. Ie origin and scale of one state affect the clipping region, at the time the clipping is evaluated, ie SetOrigin(...); ConstrainClippingRegion(...); is equivalent to ConstrainClippingRegion(...); SetOrigin(...); The current client provided clipping region at the time of PushState() always constrains the region of the new state. The previous region and the current local clipping may have their own individual origin and scale. To support all this, I needed to store origin and scale of each state on the stack, but I do cache the combined origin and scale. Caching only the combined region is not possible though. So the new implementation is slower than the previous, but more correct. * Refactored "copy constructor" from DrawState, which is not really a copy constructor anyways, made it private. It is only used by PushState(). * Removed some dead code from DrawState. All this improves Firefox redraw issues a tiny bit, but does not fix them. Either I am not covering Firefox needs with my test app, or the bug is somewhere else. At least Haiku behaves like BeOS with regard to client clipping regions and the state stack now... git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24428 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/servers/app/DrawState.cpp | 270 +++++++++++++++++----------------- src/servers/app/DrawState.h | 50 +++---- src/servers/app/View.cpp | 24 +-- 3 files changed, 171 insertions(+), 173 deletions(-) diff --git a/src/servers/app/DrawState.cpp b/src/servers/app/DrawState.cpp index de453a3dda..8cfa17054d 100644 --- a/src/servers/app/DrawState.cpp +++ b/src/servers/app/DrawState.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2005, Haiku. + * Copyright 2001-2008, Haiku. * Distributed under the terms of the MIT License. * * Authors: @@ -10,9 +10,10 @@ * Michael Pfeiffer */ -/** Data classes for working with BView states and draw parameters */ +//! Data classes for working with BView states and draw parameters +#include #include #include @@ -22,19 +23,27 @@ #include "DrawState.h" +using std::nothrow; + DrawState::DrawState() : fOrigin(0.0, 0.0), + fCombinedOrigin(0.0, 0.0), fScale(1.0), + fCombinedScale(1.0), fClippingRegion(NULL), + fHighColor((rgb_color){ 0, 0, 0, 255 }), fLowColor((rgb_color){ 255, 255, 255, 255 }), fPattern(kSolidHigh), + fDrawingMode(B_OP_COPY), fAlphaSrcMode(B_PIXEL_ALPHA), fAlphaFncMode(B_ALPHA_OVERLAY), + fPenLocation(0.0, 0.0), fPenSize(1.0), + fFontAliasing(false), fSubPixelPrecise(false), fLineCapMode(B_BUTT_CAP), @@ -46,10 +55,39 @@ DrawState::DrawState() } -DrawState::DrawState(const DrawState& from) - : fClippingRegion(NULL) +DrawState::DrawState(DrawState* from) + : fOrigin(0.0, 0.0), + fCombinedOrigin(from->fCombinedOrigin), + fScale(1.0), + fCombinedScale(from->fCombinedScale), + fClippingRegion(NULL), + + fHighColor(from->fHighColor), + fLowColor(from->fLowColor), + fPattern(from->fPattern), + + fDrawingMode(from->fDrawingMode), + fAlphaSrcMode(from->fAlphaSrcMode), + fAlphaFncMode(from->fAlphaFncMode), + + fPenLocation(from->fPenLocation), + fPenSize(from->fPenSize), + + fFont(from->fFont), + fFontAliasing(from->fFontAliasing), + + fSubPixelPrecise(from->fSubPixelPrecise), + + fLineCapMode(from->fLineCapMode), + fLineJoinMode(from->fLineJoinMode), + fMiterLimit(from->fMiterLimit), + + // Since fScale is reset to 1.0, the unscaled + // font size is the current size of the font + // (which is from->fUnscaledFontSize * from->fCombinedScale) + fUnscaledFontSize(from->fUnscaledFontSize), + fPreviousState(from) { - *this = from; } @@ -60,59 +98,10 @@ DrawState::~DrawState() } -DrawState& -DrawState::operator=(const DrawState& from) -{ - fOrigin = from.fOrigin; - fScale = from.fScale; - - if (from.fClippingRegion) { - if (fClippingRegion) - *fClippingRegion = *from.fClippingRegion; - else - fClippingRegion = new BRegion(*from.fClippingRegion); - } else { - delete fClippingRegion; - fClippingRegion = NULL; - } - - fHighColor = from.fHighColor; - fLowColor = from.fLowColor; - fPattern = from.fPattern; - - fDrawingMode = from.fDrawingMode; - fAlphaSrcMode = from.fAlphaSrcMode; - fAlphaFncMode = from.fAlphaFncMode; - - fPenLocation = from.fPenLocation; - fPenSize = from.fPenSize; - - fFont = from.fFont; - fFontAliasing = from.fFontAliasing; - - fSubPixelPrecise = from.fSubPixelPrecise; - - fLineCapMode = from.fLineCapMode; - fLineJoinMode = from.fLineJoinMode; - fMiterLimit = from.fMiterLimit; - - // Since fScale is reset to 1.0, the unscaled - // font size is the current size of the font - // (which is from->fUnscaledFontSize * from->fScale) - fUnscaledFontSize = from.fUnscaledFontSize; - fPreviousState = from.fPreviousState; - - return *this; -} - - DrawState* DrawState::PushState() { - DrawState* next = new DrawState(*this); - // this may throw an exception that our caller should handle - - next->fPreviousState = this; + DrawState* next = new (nothrow) DrawState(this); return next; } @@ -213,6 +202,14 @@ DrawState::ReadFromLink(BPrivate::LinkReceiver& link) link.Read(&fScale); link.Read(&fFontAliasing); + if (fPreviousState) { + fCombinedOrigin = fPreviousState->fCombinedOrigin + fOrigin; + fCombinedScale = fPreviousState->fCombinedScale * fScale; + } else { + fCombinedOrigin = fOrigin; + fCombinedScale = fScale; + } + fHighColor = highColor; fLowColor = lowColor; fPattern = patt; @@ -283,78 +280,115 @@ void DrawState::SetOrigin(const BPoint& origin) { fOrigin = origin; - // origin is given as a point in the - // "outer" coordinate system, therefore - // it has to be transformed - if (PreviousState() != NULL) - PreviousState()->Transform(&fOrigin); -} - -void -DrawState::OffsetOrigin(const BPoint& offset) -{ - if (PreviousState() == NULL) - fOrigin += offset; - else { - // TODO this is necessary only if offset - // is in the "outer" coordinate system - float scale = PreviousState()->Scale(); - fOrigin.x += offset.x * scale; - fOrigin.y += offset.y * scale; - } + // NOTE: the origins of earlier states are never expected to + // change, only the topmost state ever changes + if (fPreviousState) + fCombinedOrigin = fPreviousState->fCombinedOrigin + fOrigin; + else + fCombinedOrigin = fOrigin; } void DrawState::SetScale(float scale) { - // the scale is multiplied with the scale of the previous state if any - float localScale = scale; - if (PreviousState() != NULL) - localScale *= PreviousState()->Scale(); + if (fScale != scale) { + fScale = scale; - if (fScale != localScale) { - fScale = localScale; + // NOTE: the scales of earlier states are never expected to + // change, only the topmost state ever changes + if (fPreviousState) + fCombinedScale = fPreviousState->fCombinedScale * fScale; + else + fCombinedScale = fScale; // update font size - // (pen size is currently calulated on the fly) - fFont.SetSize(fUnscaledFontSize * fScale); + fFont.SetSize(fUnscaledFontSize * fCombinedScale); } } -// Transform + +void +DrawState::SetClippingRegion(const BRegion* region) +{ + if (region) { + if (fClippingRegion) + *fClippingRegion = *region; + else + fClippingRegion = new (nothrow) BRegion(*region); + } else { + delete fClippingRegion; + fClippingRegion = NULL; + } +} + + +bool +DrawState::HasClipping() const +{ + if (fClippingRegion) + return true; + if (fPreviousState) + return fPreviousState->HasClipping(); + return false; +} + + +bool +DrawState::GetCombinedClippingRegion(BRegion* region) const +{ + if (fClippingRegion) { + BRegion localTransformedClipping(*fClippingRegion); + Transform(&localTransformedClipping); + + if (fPreviousState && fPreviousState->GetCombinedClippingRegion(region)) + localTransformedClipping.IntersectWith(region); + *region = localTransformedClipping; + return true; + } else { + if (fPreviousState) + return fPreviousState->GetCombinedClippingRegion(region); + } + return false; +} + + +// #pragma mark - + + void DrawState::Transform(float* x, float* y) const { // scale relative to origin, therefore // scale first then translate to // origin - *x *= fScale; - *y *= fScale; - *x += fOrigin.x; - *y += fOrigin.y; + *x *= fCombinedScale; + *y *= fCombinedScale; + *x += fCombinedOrigin.x; + *y += fCombinedOrigin.y; } -// InverseTransform + void DrawState::InverseTransform(float* x, float* y) const { - // TODO: watch out for fScale = 0? - *x -= fOrigin.x; - *y -= fOrigin.y; - *x /= fScale; - *y /= fScale; + *x -= fCombinedOrigin.x; + *y -= fCombinedOrigin.y; + if (fCombinedScale != 0.0) { + *x /= fCombinedScale; + *y /= fCombinedScale; + } } -// Transform + void DrawState::Transform(BPoint* point) const { Transform(&(point->x), &(point->y)); } -// Transform + void DrawState::Transform(BRect* rect) const { @@ -362,13 +396,12 @@ DrawState::Transform(BRect* rect) const Transform(&(rect->right), &(rect->bottom)); } -// Transform + void DrawState::Transform(BRegion* region) const { - if (fScale == 1.0) { - if (fOrigin.x != 0.0 || fOrigin.y != 0.0) - region->OffsetBy((int32)fOrigin.x, (int32)fOrigin.y); + if (fCombinedScale == 1.0) { + region->OffsetBy(fCombinedOrigin.x, fCombinedOrigin.y); } else { // TODO: optimize some more BRegion converted; @@ -384,8 +417,8 @@ DrawState::Transform(BRegion* region) const Transform(<.x, <.y); Transform(&rb.x, &rb.y); // reset bottom right to pixel "index" - rb.x++; - rb.y++; + rb.x--; + rb.y--; // add rect to converted region // NOTE/TODO: the rect would not have to go // through the whole intersection test process, @@ -405,32 +438,7 @@ DrawState::InverseTransform(BPoint* point) const } -void -DrawState::SetClippingRegion(const BRegion* region) -{ - // reset clipping to that of previous state - // (that's the starting point) - if (PreviousState() != NULL && PreviousState()->ClippingRegion()) { - if (fClippingRegion) - *fClippingRegion = *(PreviousState()->ClippingRegion()); - else - fClippingRegion = new BRegion(*(PreviousState()->ClippingRegion())); - } else { - delete fClippingRegion; - fClippingRegion = NULL; - } - - // intersect with the clipping from the passed region - // (even if it is empty) - // passing NULL unsets this states additional region, - // it will then be the region of the previous state - if (region) { - if (fClippingRegion) - fClippingRegion->IntersectWith(region); - else - fClippingRegion = new BRegion(*region); - } -} +// #pragma mark - void @@ -490,8 +498,6 @@ DrawState::PenLocation() const void DrawState::SetPenSize(float size) { - // NOTE: since pensize is calculated on the fly, - // it is ok to set it here regardless of previous state fPenSize = size; } @@ -500,7 +506,7 @@ DrawState::SetPenSize(float size) float DrawState::PenSize() const { - float penSize = fPenSize * fScale; + float penSize = fPenSize * fCombinedScale; // NOTE: As documented in the BeBook, // pen size is never smaller than 1.0. // This is supposed to be the smallest @@ -530,7 +536,7 @@ DrawState::SetFont(const ServerFont& font, uint32 flags) if (flags == B_FONT_ALL) { fFont = font; fUnscaledFontSize = font.Size(); - fFont.SetSize(fUnscaledFontSize * fScale); + fFont.SetSize(fUnscaledFontSize * fCombinedScale); } else { // family & style if (flags & B_FONT_FAMILY_AND_STYLE) @@ -538,7 +544,7 @@ DrawState::SetFont(const ServerFont& font, uint32 flags) // size if (flags & B_FONT_SIZE) { fUnscaledFontSize = font.Size(); - fFont.SetSize(fUnscaledFontSize * fScale); + fFont.SetSize(fUnscaledFontSize * fCombinedScale); } // shear if (flags & B_FONT_SHEAR) diff --git a/src/servers/app/DrawState.h b/src/servers/app/DrawState.h index c2ee3f7e7b..e7c0be4b35 100644 --- a/src/servers/app/DrawState.h +++ b/src/servers/app/DrawState.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2005, Haiku. + * Copyright 2001-2008, Haiku. * Distributed under the terms of the MIT License. * * Authors: @@ -30,13 +30,13 @@ namespace BPrivate { class DrawState { - public: +public: DrawState(); - DrawState(const DrawState& from); +private: + DrawState(DrawState* from); +public: virtual ~DrawState(); - DrawState& operator=(const DrawState& from); - DrawState* PushState(); DrawState* PopState(); DrawState* PreviousState() const { return fPreviousState; } @@ -50,13 +50,22 @@ class DrawState { // coordinate transformation void SetOrigin(const BPoint& origin); - void OffsetOrigin(const BPoint& offset); const BPoint& Origin() const { return fOrigin; } + const BPoint& CombinedOrigin() const + { return fCombinedOrigin; } void SetScale(float scale); float Scale() const { return fScale; } + float CombinedScale() const + { return fCombinedScale; } + + // additional clipping as requested by client + void SetClippingRegion(const BRegion* region); + + bool HasClipping() const; + bool GetCombinedClippingRegion(BRegion* region) const; // coordinate transformations void Transform(float* x, float* y) const; @@ -68,14 +77,6 @@ class DrawState { void InverseTransform(BPoint* point) const; - // additional clipping as requested by client - void SetClippingRegion(const BRegion* region); - const BRegion* ClippingRegion() const - { return fClippingRegion; } -/* inline int32 CountClippingRects() const - { return fClippingRegion ? fClippingRegion.CountRects() : 0; } - inline BRect ClippingRectAt(int32 index) const;*/ - // color void SetHighColor(const rgb_color& color); const rgb_color& HighColor() const @@ -140,9 +141,11 @@ class DrawState { bool SubPixelPrecise() const { return fSubPixelPrecise; } - protected: +protected: BPoint fOrigin; + BPoint fCombinedOrigin; float fScale; + float fCombinedScale; BRegion* fClippingRegion; @@ -182,19 +185,4 @@ class DrawState { DrawState* fPreviousState; }; - -// inline implementations - -/* -// ClippingRectAt -BRect -DrawState::ClippingRectAt(int32 index) const -{ - BRect r; - if (fClippingRegion) { - r = fClippingRegion.RectAt(index); - } - return r; -}*/ - -#endif /* _DRAW_STATE_H_ */ +#endif // _DRAW_STATE_H_ diff --git a/src/servers/app/View.cpp b/src/servers/app/View.cpp index 7d6f1b797c..081c0a6bc7 100644 --- a/src/servers/app/View.cpp +++ b/src/servers/app/View.cpp @@ -440,7 +440,7 @@ View::SetDrawingOrigin(BPoint origin) fDrawState->SetOrigin(origin); // rebuild clipping - if (fDrawState->ClippingRegion()) + if (fDrawState->HasClipping()) RebuildClipping(false); } @@ -464,7 +464,7 @@ View::SetScale(float scale) fDrawState->SetScale(scale); // rebuild clipping - if (fDrawState->ClippingRegion()) + if (fDrawState->HasClipping()) RebuildClipping(false); } @@ -1169,8 +1169,11 @@ View::CopyBits(IntRect src, IntRect dst, BRegion& windowContentClipping) void View::PushState() { - fDrawState = fDrawState->PushState(); - fDrawState->SetSubPixelPrecise(fFlags & B_SUBPIXEL_PRECISE); + DrawState* newState = fDrawState->PushState(); + if (newState) { + fDrawState = newState; + fDrawState->SetSubPixelPrecise(fFlags & B_SUBPIXEL_PRECISE); + } } @@ -1523,9 +1526,10 @@ View::PrintToStream() const fScreenClipping.PrintToStream(); printf(" valid: %d\n", fScreenClippingValid); printf(" state:\n"); - printf(" user clipping: %p\n", fDrawState->ClippingRegion()); - printf(" origin: BPoint(%.1f, %.1f)\n", fDrawState->Origin().x, fDrawState->Origin().y); - printf(" scale: %.2f\n", fDrawState->Scale()); + printf(" user clipping: %d\n", fDrawState->HasClipping()); + BPoint origin = fDrawState->CombinedOrigin(); + printf(" origin: BPoint(%.1f, %.1f)\n", origin.x, origin.y); + printf(" scale: %.2f\n", fDrawState->CombinedScale()); printf("\n"); } @@ -1561,17 +1565,17 @@ View::RebuildClipping(bool deep) } // add the user clipping in case there is one - if (const BRegion* userClipping = fDrawState->ClippingRegion()) { + if (fDrawState->HasClipping()) { // NOTE: in case the user sets a user defined clipping region, // rebuilding the clipping is a bit more expensive because there // is no separate "drawing region"... on the other // hand, views for which this feature is actually used will // probably not have any children, so it is not that expensive // after all - BRegion* screenUserClipping = fWindow->GetRegion(*userClipping); + BRegion* screenUserClipping = fWindow->GetRegion(); if (!screenUserClipping) return; - fDrawState->Transform(screenUserClipping); + fDrawState->GetCombinedClippingRegion(screenUserClipping); fLocalClipping.IntersectWith(screenUserClipping); fWindow->RecycleRegion(screenUserClipping); }