← Back to team overview

kicad-developers team mailing list archive

Re: [patch] GAL pixel alignment

 

Am 2019-02-11 11:49, schrieb Tomasz Wlostowski:

OpenGL 2x Oversampling appears the same, but OpenGL with no antialiasing has caught up a lot. It’s now almost as good as 2X OS — in fact it would be usable if you wanted to switch to it for a really large design to get speed back up.

Do the fixes address the original report's different-line-thichness issues? (I think Fallback on Mac used to have similar issues, but it was so bad I never used it.) If so I’d encourage us to at least consider 5.1. I hate it when parts of the program backslide in subsequent releases.
Yes. I'm also working on the circle alignment fix and the issues
reported by Seth.

Tom

Hi Tom-

Find attached a patch for arc drawing. In Cairo, arcs in pcbnew did not show up. This may also address the drawing issue Jeff observed (although I cannot replicate it).

-S
From fc25cd92b553d9cebcfd9d377066e3a6ea40b4e5 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 11 Feb 2019 11:47:28 -0800
Subject: [PATCH] GAL: Fix arc drawing

Removes DrawArcSegment, which drew arcs whose edge was optionally
stroked (a feature that we did not use).  Fixes Cairo arc drawing issue
where arcs were not visible in pcbnew.
---
 common/gal/cairo/cairo_gal.cpp           |  49 -----------
 common/gal/opengl/opengl_gal.cpp         | 100 -----------------------
 gerbview/gerbview_painter.cpp            |   2 +-
 include/gal/cairo/cairo_gal.h            |   4 -
 include/gal/graphics_abstraction_layer.h |  18 ----
 include/gal/opengl/opengl_gal.h          |   4 -
 pcbnew/pcb_painter.cpp                   |   9 +-
 7 files changed, 7 insertions(+), 179 deletions(-)

diff --git a/common/gal/cairo/cairo_gal.cpp b/common/gal/cairo/cairo_gal.cpp
index ec12456167..a71e5c8d43 100644
--- a/common/gal/cairo/cairo_gal.cpp
+++ b/common/gal/cairo/cairo_gal.cpp
@@ -276,55 +276,6 @@ void CAIRO_GAL_BASE::DrawArc( const VECTOR2D& aCenterPoint, double aRadius, doub
 }
 
 
-void CAIRO_GAL_BASE::DrawArcSegment( const VECTOR2D& aCenterPoint, double aRadius, double aStartAngle,
-                                double aEndAngle, double aWidth )
-{
-    SWAP( aStartAngle, >, aEndAngle );
-
-    syncLineWidth();
-
-    if( isFillEnabled )
-    {
-        // Filled segments mode
-        SetLineWidth( aWidth );
-        cairo_arc( currentContext, aCenterPoint.x, aCenterPoint.y, aRadius, aStartAngle, aEndAngle );
-        cairo_set_source_rgba( currentContext, fillColor.r, fillColor.g, fillColor.b, fillColor.a );
-        cairo_stroke( currentContext );
-    }
-    else
-    {
-        double width = aWidth / 2.0;
-        VECTOR2D startPoint( cos( aStartAngle ) * aRadius,
-                             sin( aStartAngle ) * aRadius );
-        VECTOR2D endPoint( cos( aEndAngle ) * aRadius,
-                           sin( aEndAngle ) * aRadius );
-
-        cairo_save( currentContext );
-
-        cairo_set_source_rgba( currentContext, strokeColor.r, strokeColor.g, strokeColor.b, strokeColor.a );
-
-        cairo_translate( currentContext, aCenterPoint.x, aCenterPoint.y );
-
-        cairo_new_sub_path( currentContext );
-        cairo_arc( currentContext, 0, 0, aRadius - width, aStartAngle, aEndAngle );
-
-        cairo_new_sub_path( currentContext );
-        cairo_arc( currentContext, 0, 0, aRadius + width, aStartAngle, aEndAngle );
-
-        cairo_new_sub_path( currentContext );
-        cairo_arc_negative( currentContext, startPoint.x, startPoint.y, width, aStartAngle, aStartAngle + M_PI );
-
-        cairo_new_sub_path( currentContext );
-        cairo_arc( currentContext, endPoint.x, endPoint.y, width, aEndAngle, aEndAngle + M_PI );
-
-        cairo_restore( currentContext );
-        flushPath();
-    }
-
-    isElementAdded = true;
-}
-
-
 void CAIRO_GAL_BASE::DrawRectangle( const VECTOR2D& aStartPoint, const VECTOR2D& aEndPoint )
 {
     // Calculate the diagonal points
diff --git a/common/gal/opengl/opengl_gal.cpp b/common/gal/opengl/opengl_gal.cpp
index e10906f8f1..f12c999724 100644
--- a/common/gal/opengl/opengl_gal.cpp
+++ b/common/gal/opengl/opengl_gal.cpp
@@ -763,102 +763,6 @@ void OPENGL_GAL::DrawArc( const VECTOR2D& aCenterPoint, double aRadius, double a
 }
 
 
-void OPENGL_GAL::DrawArcSegment( const VECTOR2D& aCenterPoint, double aRadius, double aStartAngle,
-                                 double aEndAngle, double aWidth )
-{
-    if( aRadius <= 0 )
-    {
-        // Arcs of zero radius are a circle of aWidth diameter
-        if( aWidth > 0 )
-            DrawCircle( aCenterPoint, aWidth / 2.0 );
-
-        return;
-    }
-
-    // Swap the angles, if start angle is greater than end angle
-    SWAP( aStartAngle, >, aEndAngle );
-
-    const double alphaIncrement = calcAngleStep( aRadius );
-
-    Save();
-    currentManager->Translate( aCenterPoint.x, aCenterPoint.y, 0.0 );
-
-    if( isStrokeEnabled )
-    {
-        currentManager->Color( strokeColor.r, strokeColor.g, strokeColor.b, strokeColor.a );
-
-        double width = aWidth / 2.0;
-        VECTOR2D startPoint( cos( aStartAngle ) * aRadius,
-                             sin( aStartAngle ) * aRadius );
-        VECTOR2D endPoint( cos( aEndAngle ) * aRadius,
-                           sin( aEndAngle ) * aRadius );
-
-        drawStrokedSemiCircle( startPoint, width, aStartAngle + M_PI );
-        drawStrokedSemiCircle( endPoint, width, aEndAngle );
-
-        VECTOR2D pOuter( cos( aStartAngle ) * ( aRadius + width ),
-                         sin( aStartAngle ) * ( aRadius + width ) );
-
-        VECTOR2D pInner( cos( aStartAngle ) * ( aRadius - width ),
-                         sin( aStartAngle ) * ( aRadius - width ) );
-
-        double alpha;
-
-        for( alpha = aStartAngle + alphaIncrement; alpha <= aEndAngle; alpha += alphaIncrement )
-        {
-            VECTOR2D pNextOuter( cos( alpha ) * ( aRadius + width ),
-                                 sin( alpha ) * ( aRadius + width ) );
-            VECTOR2D pNextInner( cos( alpha ) * ( aRadius - width ),
-                                 sin( alpha ) * ( aRadius - width ) );
-
-            DrawLine( pOuter, pNextOuter );
-            DrawLine( pInner, pNextInner );
-
-            pOuter = pNextOuter;
-            pInner = pNextInner;
-        }
-
-        // Draw the last missing part
-        if( alpha != aEndAngle )
-        {
-            VECTOR2D pLastOuter( cos( aEndAngle ) * ( aRadius + width ),
-                                 sin( aEndAngle ) * ( aRadius + width ) );
-            VECTOR2D pLastInner( cos( aEndAngle ) * ( aRadius - width ),
-                                 sin( aEndAngle ) * ( aRadius - width ) );
-
-            DrawLine( pOuter, pLastOuter );
-            DrawLine( pInner, pLastInner );
-        }
-    }
-
-    if( isFillEnabled )
-    {
-        currentManager->Color( fillColor.r, fillColor.g, fillColor.b, fillColor.a );
-        SetLineWidth( aWidth );
-
-        VECTOR2D p( cos( aStartAngle ) * aRadius, sin( aStartAngle ) * aRadius );
-        double alpha;
-
-        for( alpha = aStartAngle + alphaIncrement; alpha <= aEndAngle; alpha += alphaIncrement )
-        {
-            VECTOR2D p_next( cos( alpha ) * aRadius, sin( alpha ) * aRadius );
-            DrawLine( p, p_next );
-
-            p = p_next;
-        }
-
-        // Draw the last missing part
-        if( alpha != aEndAngle )
-        {
-            VECTOR2D p_last( cos( aEndAngle ) * aRadius, sin( aEndAngle ) * aRadius );
-            DrawLine( p, p_last );
-        }
-    }
-
-    Restore();
-}
-
-
 void OPENGL_GAL::DrawRectangle( const VECTOR2D& aStartPoint, const VECTOR2D& aEndPoint )
 {
     // Compute the diagonal points of the rectangle
@@ -1645,11 +1549,7 @@ void OPENGL_GAL::drawLineQuad( const VECTOR2D& aStartPoint, const VECTOR2D& aEnd
     auto v2  = currentManager->GetTransformation() * glm::vec4( aEndPoint.x, aEndPoint.y, 0.0, 0.0 );
 
     VECTOR2D startEndVector( v2.x - v1.x, v2.y - v1.y );
-
-    double lineLength     = startEndVector.EuclideanNorm();
-
     VECTOR2D vs ( startEndVector );
-    float aspect;
 
     currentManager->Reserve( 6 );
 
diff --git a/gerbview/gerbview_painter.cpp b/gerbview/gerbview_painter.cpp
index 752b6d0e41..dbcb718302 100644
--- a/gerbview/gerbview_painter.cpp
+++ b/gerbview/gerbview_painter.cpp
@@ -322,7 +322,7 @@ void GERBVIEW_PAINTER::draw( /*const*/ GERBER_DRAW_ITEM* aItem, int aLayer )
             endAngle = 2 * M_PI;
         }
 
-        m_gal->DrawArcSegment( center, radius, startAngle, endAngle, width );
+        m_gal->DrawArc( center, radius, startAngle, endAngle );
 
         // Arc Debugging
         // m_gal->SetLineWidth( 5 );
diff --git a/include/gal/cairo/cairo_gal.h b/include/gal/cairo/cairo_gal.h
index 9eb55eadd3..2ae048a0fa 100644
--- a/include/gal/cairo/cairo_gal.h
+++ b/include/gal/cairo/cairo_gal.h
@@ -91,10 +91,6 @@ public:
     virtual void DrawArc( const VECTOR2D& aCenterPoint, double aRadius,
                           double aStartAngle, double aEndAngle ) override;
 
-    /// @copydoc GAL::DrawArcSegment()
-    virtual void DrawArcSegment( const VECTOR2D& aCenterPoint, double aRadius,
-                                 double aStartAngle, double aEndAngle, double aWidth ) override;
-
     /// @copydoc GAL::DrawRectangle()
     virtual void DrawRectangle( const VECTOR2D& aStartPoint, const VECTOR2D& aEndPoint ) override;
 
diff --git a/include/gal/graphics_abstraction_layer.h b/include/gal/graphics_abstraction_layer.h
index 0cb2ae4930..ae227fbe81 100644
--- a/include/gal/graphics_abstraction_layer.h
+++ b/include/gal/graphics_abstraction_layer.h
@@ -134,24 +134,6 @@ public:
     virtual void
     DrawArc( const VECTOR2D& aCenterPoint, double aRadius, double aStartAngle, double aEndAngle ) {};
 
-    /**
-     * @brief Draw an arc segment.
-     *
-     * This method differs from DrawArc() in what happens when fill/stroke are on or off.
-     * DrawArc() draws a "pie piece" when fill is turned on, and a thick stroke when fill is off.
-     * DrawArcSegment() with fill *on* behaves like DrawArc() with fill *off*.
-     * DrawArcSegment() with fill *off* draws the outline of what it would have drawn with fill on.
-     *
-     * @param aCenterPoint  is the center point of the arc.
-     * @param aRadius       is the arc radius.
-     * @param aStartAngle   is the start angle of the arc.
-     * @param aEndAngle     is the end angle of the arc.
-     * @param aWidth        is the thickness of the arc (pen size).
-     */
-    virtual void
-    DrawArcSegment( const VECTOR2D& aCenterPoint, double aRadius, double aStartAngle,
-                   double aEndAngle, double aWidth ) {};
-
     /**
      * @brief Draw a rectangle.
      *
diff --git a/include/gal/opengl/opengl_gal.h b/include/gal/opengl/opengl_gal.h
index d14fd0921b..322e157cba 100644
--- a/include/gal/opengl/opengl_gal.h
+++ b/include/gal/opengl/opengl_gal.h
@@ -119,10 +119,6 @@ public:
     virtual void DrawArc( const VECTOR2D& aCenterPoint, double aRadius,
                           double aStartAngle, double aEndAngle ) override;
 
-    /// @copydoc GAL::DrawArcSegment()
-    virtual void DrawArcSegment( const VECTOR2D& aCenterPoint, double aRadius,
-                                 double aStartAngle, double aEndAngle, double aWidth ) override;
-
     /// @copydoc GAL::DrawRectangle()
     virtual void DrawRectangle( const VECTOR2D& aStartPoint, const VECTOR2D& aEndPoint ) override;
 
diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp
index 8814249e79..d933096217 100644
--- a/pcbnew/pcb_painter.cpp
+++ b/pcbnew/pcb_painter.cpp
@@ -937,10 +937,13 @@ void PCB_PAINTER::draw( const DRAWSEGMENT* aSegment, int aLayer )
         break;
 
     case S_ARC:
-        m_gal->DrawArcSegment( start, aSegment->GetRadius(),
+
+        m_gal->SetLineWidth( thickness );
+        m_gal->SetIsFill( false );
+        m_gal->SetIsStroke( true );
+        m_gal->DrawArc( start, aSegment->GetRadius(),
                         DECIDEG2RAD( aSegment->GetArcAngleStart() ),
-                        DECIDEG2RAD( aSegment->GetArcAngleStart() + aSegment->GetAngle() ),
-                        thickness );
+                        DECIDEG2RAD( aSegment->GetArcAngleStart() + aSegment->GetAngle() ) );
         break;
 
     case S_CIRCLE:
-- 
2.20.1


References