* Painter switches back correctly from using B_OP_COPY for text to

using the "normal" B_OP_COPY for lines and such that works more
  like B_OP_OVER actually (for example, the slider in VLC will look
  much better, but also other stuff)
* combined Stroke and FillEllipse() into DrawEllipse() and fixed
  some longstanding issues, ellipses are now correctly placed
  (aligned) and of the correct size
* removed locking in the DrawingEngine drawing functions, since
  you need to have the DrawingEngine locked anyways for the
  clipping to stay what it is (and that's already the case elsewhere
  in the code)
* simplified ConstrainClippingRegion, the NULL version was never
  useful and also locking is removed, see above

summary: slight speed improvements, cleanup and bugfixes...


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@17329 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/servers/app/drawing/DrawingEngine.cpp

@@ -134,23 +134,11 @@ DrawingEngine::SetHWInterface(HWInterface* interface)
 
 // #pragma mark -
 
+//! the DrawingEngine needs to be locked!
 void
 DrawingEngine::ConstrainClippingRegion(const BRegion* region)
 {
-	if (Lock()) {
+	fPainter->ConstrainClipping(region);
-		if (!region) {
-//			BRegion empty;
-//			fPainter->ConstrainClipping(empty);
-			if (RenderingBuffer* buffer = fGraphicsCard->DrawingBuffer()) {
-				BRegion all;
-				all.Include(BRect(0, 0, buffer->Width() - 1, buffer->Height() - 1));
-				fPainter->ConstrainClipping(all);
-			}
-		} else {
-			fPainter->ConstrainClipping(*region);
-		}
-		Unlock();
-	}
 }
 
 // SuspendAutoSync
@@ -518,19 +506,10 @@ DrawingEngine::DrawEllipse(BRect r, const DrawState *d, bool filled)
 			extend_by_stroke_width(clipped, d);
 		clipped = fPainter->ClipRect(clipped);
 		if (clipped.IsValid()) {
-			fGraphicsCard->HideSoftwareCursor(r);
+			fGraphicsCard->HideSoftwareCursor(clipped);
 	
 			fPainter->SetDrawState(d);
-	
+			fPainter->DrawEllipse(r, filled);
-			float xRadius = r.Width() / 2.0;
-			float yRadius = r.Height() / 2.0;
-			BPoint center(r.left + xRadius,
-						  r.top + yRadius);
-
-			if (filled)
-				fPainter->FillEllipse(center, xRadius, yRadius);
-			else
-				fPainter->StrokeEllipse(center, xRadius, yRadius);
 	
 			fGraphicsCard->Invalidate(clipped);
 			fGraphicsCard->ShowSoftwareCursor();
@@ -573,6 +552,31 @@ DrawingEngine::StrokePoint(const BPoint& pt, const RGBColor &color)
 	StrokeLine(pt, pt, color);
 }
 
+// StrokeLine
+//
+// * this function is only used by Decorators
+// * it assumes a one pixel wide line
+void
+DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, const RGBColor &color)
+{
+	if (Lock()) {
+		BRect touched(start, end);
+		make_rect_valid(touched);
+		touched = fPainter->ClipRect(touched);
+		fGraphicsCard->HideSoftwareCursor(touched);
+
+		if (!fPainter->StraightLine(start, end, color.GetColor32())) {
+			DrawState context;
+			context.SetHighColor(color);
+			context.SetDrawingMode(B_OP_OVER);
+			StrokeLine(start, end, &context);
+		} else {
+			fGraphicsCard->Invalidate(touched);
+		}
+		fGraphicsCard->ShowSoftwareCursor();
+		Unlock();
+	}
+}
 
 // this function is used to draw a one pixel wide rect
 void
@@ -671,24 +675,20 @@ DrawingEngine::FillRegion(BRegion& r, const RGBColor& color)
 void
 DrawingEngine::StrokeRect(BRect r, const DrawState *d)
 {
-	if (Lock()) {
+	// support invalid rects
-		// support invalid rects
+	make_rect_valid(r);
-		make_rect_valid(r);
+	BRect clipped(r);
-		BRect clipped(r);
+	extend_by_stroke_width(clipped, d);
-		extend_by_stroke_width(clipped, d);
+	clipped = fPainter->ClipRect(clipped);
-		clipped = fPainter->ClipRect(clipped);
+	if (clipped.IsValid()) {
-		if (clipped.IsValid()) {
-	
-			fGraphicsCard->HideSoftwareCursor(clipped);
-	
-			fPainter->SetDrawState(d);
-			fPainter->StrokeRect(r);
-	
-			fGraphicsCard->Invalidate(clipped);
-			fGraphicsCard->ShowSoftwareCursor();
-		}
 
-		Unlock();
+		fGraphicsCard->HideSoftwareCursor(clipped);
+
+		fPainter->SetDrawState(d);
+		fPainter->StrokeRect(r);
+
+		fGraphicsCard->Invalidate(clipped);
+		fGraphicsCard->ShowSoftwareCursor();
 	}
 }
 
@@ -804,9 +804,6 @@ void
 DrawingEngine::DrawRoundRect(BRect r, float xrad, float yrad,
 	const DrawState* d, bool filled)
 {
-	if (!Lock())
-		return;
-
 	// NOTE: the stroke does not extend past "r" in R5,
 	// though I consider this unexpected behaviour.
 	make_rect_valid(r);
@@ -821,8 +818,6 @@ DrawingEngine::DrawRoundRect(BRect r, float xrad, float yrad,
 		fGraphicsCard->Invalidate(touched);
 		fGraphicsCard->ShowSoftwareCursor();
 	}
-
-	Unlock();
 }
 
 
@@ -831,9 +826,6 @@ DrawingEngine::DrawShape(const BRect& bounds, int32 opCount,
 	const uint32* opList, int32 ptCount, const BPoint* ptList,
 	const DrawState* d, bool filled)
 {
-	if (!Lock())
-		return;
-
 	fGraphicsCard->HideSoftwareCursor();
 
 	fPainter->SetDrawState(d);
@@ -843,8 +835,6 @@ DrawingEngine::DrawShape(const BRect& bounds, int32 opCount,
 
 	fGraphicsCard->Invalidate(touched);
 	fGraphicsCard->ShowSoftwareCursor();
-
-	Unlock();
 }
 
 
@@ -852,51 +842,21 @@ void
 DrawingEngine::DrawTriangle(BPoint* pts, const BRect& bounds,
 	const DrawState* d, bool filled)
 {
-	if (Lock()) {
+	BRect clipped(bounds);
-		BRect clipped(bounds);
+	if (!filled)
-		if (!filled)
+		extend_by_stroke_width(clipped, d);
-			extend_by_stroke_width(clipped, d);
+	clipped = fPainter->ClipRect(clipped);
-		clipped = fPainter->ClipRect(clipped);
+	if (clipped.IsValid()) {
-		if (clipped.IsValid()) {
+		fGraphicsCard->HideSoftwareCursor(clipped);
-			fGraphicsCard->HideSoftwareCursor(clipped);
 
-			fPainter->SetDrawState(d);
+		fPainter->SetDrawState(d);
-			if (filled)
+		if (filled)
-				fPainter->FillTriangle(pts[0], pts[1], pts[2]);
+			fPainter->FillTriangle(pts[0], pts[1], pts[2]);
-			else
+		else
-				fPainter->StrokeTriangle(pts[0], pts[1], pts[2]);
+			fPainter->StrokeTriangle(pts[0], pts[1], pts[2]);
 
-			fGraphicsCard->Invalidate(clipped);
+		fGraphicsCard->Invalidate(clipped);
-			fGraphicsCard->ShowSoftwareCursor();
-		}
-
-		Unlock();
-	}
-}
-
-// StrokeLine
-//
-// * this function is only used by Decorators
-// * it assumes a one pixel wide line
-void
-DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, const RGBColor &color)
-{
-	if (Lock()) {
-		BRect touched(start, end);
-		make_rect_valid(touched);
-		touched = fPainter->ClipRect(touched);
-		fGraphicsCard->HideSoftwareCursor(touched);
-
-		if (!fPainter->StraightLine(start, end, color.GetColor32())) {
-			DrawState context;
-			context.SetHighColor(color);
-			context.SetDrawingMode(B_OP_OVER);
-			StrokeLine(start, end, &context);
-		} else {
-			fGraphicsCard->Invalidate(touched);
-		}
 		fGraphicsCard->ShowSoftwareCursor();
-		Unlock();
 	}
 }
 
@@ -904,22 +864,18 @@ DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, const RGBColor
 void
 DrawingEngine::StrokeLine(const BPoint &start, const BPoint &end, DrawState* context)
 {
-	if (Lock()) {
+	BRect touched(start, end);
-		BRect touched(start, end);
+	make_rect_valid(touched);
-		make_rect_valid(touched);
+	extend_by_stroke_width(touched, context);
-		extend_by_stroke_width(touched, context);
+	touched = fPainter->ClipRect(touched);
-		touched = fPainter->ClipRect(touched);
+	if (touched.IsValid()) {
-		if (touched.IsValid()) {
+		fGraphicsCard->HideSoftwareCursor(touched);
-			fGraphicsCard->HideSoftwareCursor(touched);
 
-			fPainter->SetDrawState(context);
+		fPainter->SetDrawState(context);
-			touched = fPainter->StrokeLine(start, end);
+		touched = fPainter->StrokeLine(start, end);
 
-			fGraphicsCard->Invalidate(touched);
+		fGraphicsCard->Invalidate(touched);
-			fGraphicsCard->ShowSoftwareCursor();
+		fGraphicsCard->ShowSoftwareCursor();
-		}
-
-		Unlock();
 	}
 }
 
@@ -931,51 +887,47 @@ DrawingEngine::StrokeLineArray(int32 numLines,
 	if (!d || !linedata || numLines <= 0)
 		return;
 
-	if (Lock()) {
+	// figure out bounding box for line array
-		// figure out bounding box for line array
+	const LineArrayData *data = (const LineArrayData *)&(linedata[0]);
-		const LineArrayData *data = (const LineArrayData *)&(linedata[0]);
+	BRect touched(min_c(data->pt1.x, data->pt2.x),
-		BRect touched(min_c(data->pt1.x, data->pt2.x),
+				  min_c(data->pt1.y, data->pt2.y),
-					  min_c(data->pt1.y, data->pt2.y),
+				  max_c(data->pt1.x, data->pt2.x),
-					  max_c(data->pt1.x, data->pt2.x),
+				  max_c(data->pt1.y, data->pt2.y));
-					  max_c(data->pt1.y, data->pt2.y));
+
+	for (int32 i = 1; i < numLines; i++) {
+		data = (const LineArrayData *)&(linedata[i]);
+		BRect box(min_c(data->pt1.x, data->pt2.x),
+				  min_c(data->pt1.y, data->pt2.y),
+				  max_c(data->pt1.x, data->pt2.x),
+				  max_c(data->pt1.y, data->pt2.y));
+		touched = touched | box;
+	}
+	extend_by_stroke_width(touched, d);
+	touched = fPainter->ClipRect(touched);
+	if (touched.IsValid()) {
+		fGraphicsCard->HideSoftwareCursor(touched);
+
+		data = (const LineArrayData *)&(linedata[0]);
+
+		DrawState context;
+		context.SetDrawingMode(d->GetDrawingMode());
+		context.SetLowColor(d->LowColor());
+		context.SetHighColor(data->color);
+		context.SetPenSize(d->PenSize());
+			// pen size is already correctly scaled
+
+		fPainter->SetDrawState(&context);
+
+		fPainter->StrokeLine(data->pt1, data->pt2);
 
 		for (int32 i = 1; i < numLines; i++) {
 			data = (const LineArrayData *)&(linedata[i]);
-			BRect box(min_c(data->pt1.x, data->pt2.x),
+			fPainter->SetHighColor(data->color);
-					  min_c(data->pt1.y, data->pt2.y),
-					  max_c(data->pt1.x, data->pt2.x),
-					  max_c(data->pt1.y, data->pt2.y));
-			touched = touched | box;
-		}
-		extend_by_stroke_width(touched, d);
-		touched = fPainter->ClipRect(touched);
-		if (touched.IsValid()) {
-			fGraphicsCard->HideSoftwareCursor(touched);
-
-			data = (const LineArrayData *)&(linedata[0]);
-
-			DrawState context;
-			context.SetDrawingMode(d->GetDrawingMode());
-			context.SetLowColor(d->LowColor());
-			context.SetHighColor(data->color);
-			context.SetPenSize(d->PenSize());
-				// pen size is already correctly scaled
-
-			fPainter->SetDrawState(&context);
-
 			fPainter->StrokeLine(data->pt1, data->pt2);
-
-			for (int32 i = 1; i < numLines; i++) {
-				data = (const LineArrayData *)&(linedata[i]);
-				fPainter->SetHighColor(data->color);
-				fPainter->StrokeLine(data->pt1, data->pt2);
-			}
-
-			fGraphicsCard->Invalidate(touched);
-			fGraphicsCard->ShowSoftwareCursor();
 		}
 
-		Unlock();
+		fGraphicsCard->Invalidate(touched);
+		fGraphicsCard->ShowSoftwareCursor();
 	}
 }
 
@@ -990,8 +942,8 @@ DrawingEngine::DrawString(const char* string, int32 length,
 	FontLocker locker(d);
 
 	BPoint penLocation = pt;
-	if (Lock()) {
+
-		fPainter->SetDrawState(d, true);
+	fPainter->SetDrawState(d, true);
 //bigtime_t now = system_time();
 // TODO: BoundingBox is quite slow!! Optimizing it will be beneficial.
 // Cursiously, the DrawString after it is actually faster!?!
@@ -1000,22 +952,21 @@ DrawingEngine::DrawString(const char* string, int32 length,
 // in case we don't have one.
 // TODO: Watch out about penLocation and use Painter::PenLocation() when
 // not using BoundindBox anymore.
-		BRect b = fPainter->BoundingBox(string, length, pt, &penLocation, delta);
+	BRect b = fPainter->BoundingBox(string, length, pt, &penLocation, delta);
-		// stop here if we're supposed to render outside of the clipping
+	// stop here if we're supposed to render outside of the clipping
-		b = fPainter->ClipRect(b);
+	b = fPainter->ClipRect(b);
-		if (b.IsValid()) {
+	if (b.IsValid()) {
 //printf("bounding box '%s': %lld µs\n", string, system_time() - now);
-			fGraphicsCard->HideSoftwareCursor(b);
+		fGraphicsCard->HideSoftwareCursor(b);
 
 //now = system_time();
-			BRect touched = fPainter->DrawString(string, length, pt, delta);
+		BRect touched = fPainter->DrawString(string, length, pt, delta);
 //printf("drawing string: %lld µs\n", system_time() - now);
 
-			fGraphicsCard->Invalidate(touched);
+		fGraphicsCard->Invalidate(touched);
-			fGraphicsCard->ShowSoftwareCursor();
+		fGraphicsCard->ShowSoftwareCursor();
-		}
-		Unlock();
 	}
+
 	return penLocation;
 }
 

src/servers/app/drawing/Painter/Painter.cpp

@@ -215,7 +215,7 @@ Painter::SetDrawState(const DrawState* data, bool updateFont)
 	SetHighColor(data->HighColor().GetColor32());
 	SetLowColor(data->LowColor().GetColor32());
 
-	if (updateDrawingMode)
+	if (updateDrawingMode || fPixelFormat->UsesOpCopyForText())
 		_UpdateDrawingMode();
 }
 
@@ -223,10 +223,14 @@ Painter::SetDrawState(const DrawState* data, bool updateFont)
 
 // ConstrainClipping
 void
-Painter::ConstrainClipping(const BRegion& region)
+Painter::ConstrainClipping(const BRegion* region)
 {
-	*fClippingRegion = region;
+	*fClippingRegion = *region;
 	fValidClipping = fClippingRegion->Frame().IsValid();
+// TODO: would be nice if we didn't need to copy a region
+// for *each* drawing command...
+//fBaseRenderer->set_clipping_region(const_cast<BRegion*>(region));
+//fValidClipping = region->Frame().IsValid();
 }
 
 // SetHighColor
@@ -829,18 +833,67 @@ Painter::FillRoundRect(const BRect& r, float xRadius, float yRadius) const
 	return _FillPath(rect);
 }
 									
-// StrokeEllipse
+// DrawEllipse
 BRect
-Painter::StrokeEllipse(BPoint center, float xRadius, float yRadius) const
+Painter::DrawEllipse(BRect r, bool fill) const
 {
-	return _DrawEllipse(center, xRadius, yRadius, false);
+	CHECK_CLIPPING
-}
 
-// FillEllipse
+	if (!fSubpixelPrecise) {
-BRect
+		// align rect to pixels
-Painter::FillEllipse(BPoint center, float xRadius, float yRadius) const
+		align_rect_to_pixels(&r);
-{
+		// account for "pixel index" versus "pixel area"
-	return _DrawEllipse(center, xRadius, yRadius, true);
+		r.right++;
+		r.bottom++;
+		if (!fill && fmodf(fPenSize, 2.0) != 0.0) {
+			// align the stroke
+			r.InsetBy(0.5, 0.5);
+		}
+	}
+
+	float xRadius = r.Width() / 2.0;
+	float yRadius = r.Height() / 2.0;
+	BPoint center(r.left + xRadius,
+				  r.top + yRadius);
+
+	int32 divisions = (int32)max_c(12, (xRadius + yRadius + 2 * fPenSize) * PI / 2);
+
+	if (fill) {
+		agg::ellipse path(center.x, center.y, xRadius, yRadius, divisions);
+
+		return _FillPath(path);
+	} else {
+		// NOTE: This implementation might seem a little strange, but it makes
+		// stroked ellipses look like on R5. A more correct way would be to use
+		// _StrokePath(), but it currently has its own set of problems with narrow
+		// ellipses (for small xRadii or yRadii).
+		float inset = fPenSize / 2.0;
+		agg::ellipse inner(center.x, center.y,
+						   max_c(0.0, xRadius - inset),
+						   max_c(0.0, yRadius - inset),
+						   divisions);
+		agg::ellipse outer(center.x, center.y,
+						   xRadius + inset,
+						   yRadius + inset,
+						   divisions);
+
+		fRasterizer->reset();
+		fRasterizer->add_path(outer);
+		fRasterizer->add_path(inner);
+
+		// make the inner ellipse work as a hole
+		fRasterizer->filling_rule(agg::fill_even_odd);
+
+		if (fPenSize > 4)
+			agg::render_scanlines(*fRasterizer, *fPackedScanline, *fRenderer);
+		else
+			agg::render_scanlines(*fRasterizer, *fUnpackedScanline, *fRenderer);
+
+		// reset to default
+		fRasterizer->filling_rule(agg::fill_non_zero);
+
+		return _Clipped(_BoundingBox(outer));
+	}
 }
 
 // StrokeArc
@@ -1120,16 +1173,20 @@ Painter::_UpdateDrawingMode(bool drawingText)
 	// instead. If we have a B_SOLID_* pattern, we can actually use
 	// the color in the renderer and special versions of drawing modes
 	// that don't use PatternHandler and are more efficient. This
-	// has been implemented for B_OP_COPY as of now, the last parameter
+	// has been implemented for B_OP_COPY and a couple others (the
-	// to DrawingModeFor() is a flag if a special "solid" drawing mode
+	// DrawingMode*Solid ones) as of now. The PixelFormat knows the
-	// should be used if available. In this case, _SetRendererColor()
+	// PatternHandler and makes its decision based on the pattern.
+	// The last parameter to SetDrawingMode() is a flag if a special
+	// for when Painter is used to draw text. In this case, another
+	// special version of B_OP_COPY is used that acts like R5 in that
+	// anti-aliased pixel are not rendered against the actual background
+	// but the current low color instead. This way, the frame buffer
+	// doesn't need to be read.
+	// When a solid pattern is used, _SetRendererColor()
 	// has to be called so that all internal colors in the renderes
 	// are up to date for use by the solid drawing mode version.
-	if (fPixelFormat) {
+	fPixelFormat->SetDrawingMode(fDrawingMode, fAlphaSrcMode,
-		fPixelFormat->SetDrawingMode(fDrawingMode, fAlphaSrcMode,
+								 fAlphaFncMode, drawingText);
-									 fAlphaFncMode, drawingText);
-	}
-		
 }
 
 // _SetRendererColor
@@ -1156,11 +1213,11 @@ Painter::_SetRendererColor(const rgb_color& color) const
 								   color.blue / 255.0,
 								   color.alpha / 255.0));
 // TODO: bitmap fonts not yet correctly setup in AGGTextRenderer
-//	if (RendererBin)
+//	if (fRendererBin)
-//		RendererBin->color(agg::rgba(color.red / 255.0,
+//		fRendererBin->color(agg::rgba(color.red / 255.0,
-//									 color.green / 255.0,
+//									  color.green / 255.0,
-//									 color.blue / 255.0,
+//									  color.blue / 255.0,
-//									 color.alpha / 255.0));
+//									  color.alpha / 255.0));
 
 }
 
@@ -1190,59 +1247,6 @@ Painter::_DrawTriangle(BPoint pt1, BPoint pt2, BPoint pt3, bool fill) const
 		return _StrokePath(path);
 }
 
-// _DrawEllipse
-inline BRect
-Painter::_DrawEllipse(BPoint center, float xRadius, float yRadius,
-					  bool fill) const
-{
-	CHECK_CLIPPING
-
-	// TODO: I think the conversion and the offset of
-	// pixel centers might not be correct here, and it
-	// might even be necessary to treat Fill and Stroke
-	// differently, as with Fill-/StrokeRect().
-	_Transform(&center);
-
-	int32 divisions = (int32)max_c(12, (xRadius + yRadius + 2 * fPenSize) * PI / 2);
-
-	if (fill) {
-		agg::ellipse path(center.x, center.y, xRadius, yRadius, divisions);
-
-		return _FillPath(path);
-	} else {
-		// NOTE: This implementation might seem a little strange, but it makes
-		// stroked ellipses look like on R5. A more correct way would be to use
-		// _StrokePath(), but it currently has its own set of problems with narrow
-		// ellipses (for small xRadii or yRadii).
-		float inset = fPenSize / 2.0;
-		agg::ellipse inner(center.x, center.y,
-						   max_c(0.0, xRadius - inset),
-						   max_c(0.0, yRadius - inset),
-						   divisions);
-		agg::ellipse outer(center.x, center.y,
-						   xRadius + inset,
-						   yRadius + inset,
-						   divisions);
-
-		fRasterizer->reset();
-		fRasterizer->add_path(outer);
-		fRasterizer->add_path(inner);
-
-		// make the inner ellipse work as a hole
-		fRasterizer->filling_rule(agg::fill_even_odd);
-
-		if (fPenSize > 4)
-			agg::render_scanlines(*fRasterizer, *fPackedScanline, *fRenderer);
-		else
-			agg::render_scanlines(*fRasterizer, *fUnpackedScanline, *fRenderer);
-
-		// reset to default
-		fRasterizer->filling_rule(agg::fill_non_zero);
-
-		return _Clipped(_BoundingBox(outer));
-	}
-}
-
 // copy_bitmap_row_cmap8_copy
 static inline void
 copy_bitmap_row_cmap8_copy(uint8* dst, const uint8* src, int32 numPixels,

src/servers/app/drawing/Painter/Painter.h

@@ -57,7 +57,7 @@ class Painter {
 			void				AttachToBuffer(RenderingBuffer* buffer);
 			void				DetachFromBuffer();
 
-			void				ConstrainClipping(const BRegion& region);
+			void				ConstrainClipping(const BRegion* region);
 			const BRegion*		ClippingRegion() const
 									{ return fClippingRegion; }
 
@@ -147,13 +147,8 @@ class Painter {
 												float yRadius) const;
 
 								// ellipses
-			BRect				StrokeEllipse(	BPoint center,
+			BRect				DrawEllipse(	BRect r,
-												float xRadius,
+												bool filled) const;
-												float yRadius) const;
-
-			BRect				FillEllipse(	BPoint center,
-												float xRadius,
-												float yRadius) const;
 
 								// arcs
 			BRect				StrokeArc(		BPoint center,
@@ -217,10 +212,6 @@ class Painter {
 												BPoint pt2,
 												BPoint pt3,
 												bool fill) const;
-			BRect				_DrawEllipse(	BPoint center,
-												float xRadius,
-												float yRadius,
-												bool fill) const;
 
 			void				_DrawBitmap(	const agg::rendering_buffer& srcBuffer,
 												color_space format,