[haiku-commits] Re: haiku: hrev47230 - in src: servers/app/drawing tests/servers/app/transformation
- From: Stephan Aßmus <superstippi@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 19 May 2014 14:54:32 +0200
Hi Adrien,
Am 19.05.2014 14:18, schrieb pulkomandy@xxxxxxxxxxxxx:
############################################################################
Commit: 79eb23a82dfd137b0a8f2a70058463f6d7a3b476
URL: http://cgit.haiku-os.org/haiku/commit/?id=79eb23a
Author: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Date: Mon May 19 11:18:45 2014 UTC
Fix handling of filled rectangles with transforms.
The DrawingEngine didn't properly make a distinction between the
rectangle being filled and the damaged region on screen. This led to
unexpected results when using BAffineTransform.
----------------------------------------------------------------------------
diff --git a/src/servers/app/drawing/DrawingEngine.cpp
b/src/servers/app/drawing/DrawingEngine.cpp
index e37ad03..a07a255 100644
--- a/src/servers/app/drawing/DrawingEngine.cpp
+++ b/src/servers/app/drawing/DrawingEngine.cpp
@@ -963,49 +963,62 @@ DrawingEngine::FillRect(BRect r)
ASSERT_PARALLEL_LOCKED();
make_rect_valid(r);
- r = fPainter->TransformAlignAndClipRect(r);
- if (!r.IsValid())
+ r.left = floorf(r.left);
+ r.top = floorf(r.top);
+ r.right = ceilf(r.right);
+ r.bottom = ceilf(r.bottom);
+
+ BRect dirty = fPainter->TransformAndClipRect(r);
+ if (!dirty.IsValid())
return;
I don't understand why you do it differently than I proposed in IRC. I
said the method TransformAlignAndClipRect() needs to be split up, so one
can use the "AlignRect" and "TransformAndClipRect" functionality
separately. Now you have not only duplicated the alignment code in two
places, you also introduced a bug: The rect is not supposed to be
aligned in certain circumstances, namely when sub-pixel-precise drawing
is used.
AutoFloatingOverlaysHider overlaysHider(fGraphicsCard, r);
Here you need to pass "dirty", since the overlays need to be removed
where the drawing is going to happen.
bool doInSoftware = true;
- if ((r.Width() + 1) * (r.Height() + 1) > 100.0) {
- // try hardware optimized version first
- // if the rect is large enough
- if ((fAvailableHWAccleration & HW_ACC_FILL_REGION) != 0) {
- if (fPainter->Pattern() == B_SOLID_HIGH
- && (fPainter->DrawingMode() == B_OP_COPY
- || fPainter->DrawingMode() ==
B_OP_OVER)) {
- BRegion region(r);
-
region.IntersectWith(fPainter->ClippingRegion());
- fGraphicsCard->FillRegion(region,
fPainter->HighColor(),
- fSuspendSyncLevel == 0 ||
overlaysHider.WasHidden());
- doInSoftware = false;
- } else if (fPainter->Pattern() == B_SOLID_LOW
- && fPainter->DrawingMode() ==
B_OP_COPY) {
- BRegion region(r);
-
region.IntersectWith(fPainter->ClippingRegion());
- fGraphicsCard->FillRegion(region,
fPainter->LowColor(),
- fSuspendSyncLevel == 0 ||
overlaysHider.WasHidden());
- doInSoftware = false;
+
+ if (fPainter->IsIdentityTransform())
+ {
+ // TODO the accelerated code path may also be used for
transforms that
+ // only scale and translate (but don't shear or rotate).
+
+ if ((r.Width() + 1) * (r.Height() + 1) > 100.0) {
+ // try hardware optimized version first
+ // if the rect is large enough
+ if ((fAvailableHWAccleration & HW_ACC_FILL_REGION) !=
0) {
+ if (fPainter->Pattern() == B_SOLID_HIGH
+ && (fPainter->DrawingMode() == B_OP_COPY
+ || fPainter->DrawingMode() ==
B_OP_OVER)) {
+ BRegion region(r);
+
region.IntersectWith(fPainter->ClippingRegion());
+ fGraphicsCard->FillRegion(region,
fPainter->HighColor(),
+ fSuspendSyncLevel == 0 ||
overlaysHider.WasHidden());
+ doInSoftware = false;
+ } else if (fPainter->Pattern() == B_SOLID_LOW
+ && fPainter->DrawingMode() ==
B_OP_COPY) {
+ BRegion region(r);
+
region.IntersectWith(fPainter->ClippingRegion());
+ fGraphicsCard->FillRegion(region,
fPainter->LowColor(),
+ fSuspendSyncLevel == 0 ||
overlaysHider.WasHidden());
+ doInSoftware = false;
+ }
}
}
- }
- if (doInSoftware && (fAvailableHWAccleration & HW_ACC_INVERT_REGION) != 0
- && fPainter->Pattern() == B_SOLID_HIGH
- && fPainter->DrawingMode() == B_OP_INVERT) {
- BRegion region(r);
- region.IntersectWith(fPainter->ClippingRegion());
- fGraphicsCard->InvertRegion(region);
- doInSoftware = false;
+ if (doInSoftware
+ && (fAvailableHWAccleration & HW_ACC_INVERT_REGION) != 0
+ && fPainter->Pattern() == B_SOLID_HIGH
+ && fPainter->DrawingMode() == B_OP_INVERT) {
+ BRegion region(r);
+ region.IntersectWith(fPainter->ClippingRegion());
+ fGraphicsCard->InvertRegion(region);
+ doInSoftware = false;
+ }
}
if (doInSoftware)
fPainter->FillRect(r);
- _CopyToFront(r);
+ _CopyToFront(dirty);
}
@@ -1015,15 +1028,20 @@ DrawingEngine::FillRect(BRect r, const BGradient&
gradient)
ASSERT_PARALLEL_LOCKED();
make_rect_valid(r);
- r = fPainter->TransformAlignAndClipRect(r);
- if (!r.IsValid())
+ r.left = floorf(r.left);
+ r.top = floorf(r.top);
+ r.right = ceilf(r.right);
+ r.bottom = ceilf(r.bottom);
+
+ BRect dirty = fPainter->TransformAndClipRect(r);
+ if (!dirty.IsValid())
return;
- AutoFloatingOverlaysHider overlaysHider(fGraphicsCard, r);
+ AutoFloatingOverlaysHider overlaysHider(fGraphicsCard, dirty);
You even did it correctly here.
fPainter->FillRect(r, gradient);
- _CopyToFront(r);
+ _CopyToFront(dirty);
}
And I notice from the patch that TransformAndClipRect() is in fact
available separately. So the patch just needed a separate treatment of
dirty rect and drawing rect, plus what you did for the possible
acceleration test.
Best regards,
-Stephan
Other related posts:
