kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

Le 29/09/2017 à 01:33, Jon Evans a écrit :
> Hi all,
> 
> The COLOR4D_PICKER_DLG has not been working on macOS [1].
> 
> Through some experimentation, I found that drawing to a wxBitmap in createRGBBitmap() /
> createHSVBitmap() just doesn't work on macOS.  I am not sure how to fix it, but I did find out that
> drawing to a wxImage works just fine, so in the attached patch I switch those two methods to use
> wxImage.
> 
> If anyone can figure out why the wxBitmap stuff works on Windows/Linux but not macOS, maybe there is
> a simpler way to fix this, but if not, the patch could be a good workaround (maybe it's a wx bug?)
> 
> -Jon

Thanks Jon for having a look into this issue.
Using alpha channel in wxImage+wxBitmap creates issues( not visible cursors and axis outside the
palettes, and incorrect color of cursors)

I removed the alpha channel.

Please, can you test this patch on macOS ?

Thanks.


-- 
Jean-Pierre CHARRAS

From 2364afe74b7b1794728d7d8334add78df8c3374d Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Thu, 28 Sep 2017 19:22:27 -0400
Subject: [PATCH] Use wxImage instead of wxBitmap in COLOR4D picker (Fixes
 LP:1718389)

---
 common/widgets/color4Dpickerdlg.cpp | 78 ++++++++++++++-----------------------
 common/widgets/color4Dpickerdlg.h   |  4 +-
 2 files changed, 32 insertions(+), 50 deletions(-)

diff --git a/common/widgets/color4Dpickerdlg.cpp b/common/widgets/color4Dpickerdlg.cpp
index 39068ef..deb3e88 100644
--- a/common/widgets/color4Dpickerdlg.cpp
+++ b/common/widgets/color4Dpickerdlg.cpp
@@ -184,21 +184,14 @@ void COLOR4D_PICKER_DLG::initDefinedColors()
 
 void COLOR4D_PICKER_DLG::createRGBBitmap()
 {
-    wxMemoryDC bitmapDC;
     wxSize bmsize = m_RgbBitmap->GetSize();
     int half_size = std::min( bmsize.x, bmsize.y )/2;
-    m_bitmapRGB = new wxBitmap( bmsize );
-    bitmapDC.SelectObject( *m_bitmapRGB );
-    wxPen pen;
-
-    // clear background (set the window bg color)
-    wxBrush bgbrush( GetBackgroundColour() );
-    bitmapDC.SetBackground( bgbrush );
-    bitmapDC.Clear();
+    m_bitmapRGB = new wxImage( bmsize );
 
-    // Use Y axis from bottom to top and origin to center
-    bitmapDC.SetAxisOrientation( true, true );
-    bitmapDC.SetDeviceOrigin( half_size, half_size );
+    // clear background
+    wxColor bgcolor( GetBackgroundColour() );
+    wxRect rect( 0, 0, bmsize.x, bmsize.y );
+    m_bitmapRGB->SetRGB( rect, bgcolor.Red(), bgcolor.Green(), bgcolor.Blue() );
 
     // Reserve room to draw cursors inside the bitmap
     half_size -= m_cursorsSize/2;
@@ -219,9 +212,9 @@ void COLOR4D_PICKER_DLG::createRGBBitmap()
         {
             color.r = inc * yy;
 
-            pen.SetColour( color.ToColour() );
-            bitmapDC.SetPen( pen );
-            bitmapDC.DrawPoint( xx, yy - (slope*xx) );
+            wxColor c = color.ToColour();
+            m_bitmapRGB->SetRGB( xx + half_size, ( -yy + (slope*xx) ) + half_size,
+                                 c.Red(), c.Green(), c.Blue() );
         }
     }
 
@@ -235,9 +228,9 @@ void COLOR4D_PICKER_DLG::createRGBBitmap()
         {
             color.r = inc * yy;
 
-            pen.SetColour( color.ToColour() );
-            bitmapDC.SetPen( pen );
-            bitmapDC.DrawPoint( -xx, yy - (slope*xx) );
+            wxColor c = color.ToColour();
+            m_bitmapRGB->SetRGB( -xx + half_size, ( -yy + (slope*xx) ) + half_size,
+                                 c.Red(), c.Green(), c.Blue() );
         }
     }
 
@@ -251,16 +244,17 @@ void COLOR4D_PICKER_DLG::createRGBBitmap()
         {
             color.b = inc * yy;
 
-            pen.SetColour( color.ToColour() );
-            bitmapDC.SetPen( pen );
-
             // Mapping the xx, yy color axis to draw coordinates is more tricky than previously
             // in DC coordinates:
             // the blue axis is the (0, 0) to half_size, (-yy - SLOPE_AXIS)
             // the green axis is the (0, 0) to - half_size, (-yy - SLOPE_AXIS)
             int drawX = -xx + yy;
-            int drawY = - std::min( xx,yy ) * 0.9;
-            bitmapDC.DrawPoint( drawX, drawY - std::abs( slope*drawX ) );
+            int drawY = std::min( xx,yy ) * 0.9;
+
+            wxColor c = color.ToColour();
+            m_bitmapRGB->SetRGB( drawX + half_size,
+                                 drawY + std::abs( slope*drawX ) + half_size,
+                                 c.Red(), c.Green(), c.Blue() );
         }
     }
 }
@@ -268,22 +262,15 @@ void COLOR4D_PICKER_DLG::createRGBBitmap()
 
 void COLOR4D_PICKER_DLG::createHSVBitmap()
 {
-    wxMemoryDC bitmapDC;
     wxSize bmsize = m_HsvBitmap->GetSize();
     int half_size = std::min( bmsize.x, bmsize.y )/2;
     delete m_bitmapHSV;
-    m_bitmapHSV = new wxBitmap( bmsize );
-    bitmapDC.SelectObject( *m_bitmapHSV );
-    wxPen pen;
+    m_bitmapHSV = new wxImage( bmsize );
 
-    // clear background (set the window bd color)
-    wxBrush bgbrush( GetBackgroundColour() );
-    bitmapDC.SetBackground( bgbrush );
-    bitmapDC.Clear();
-
-    // Use Y axis from bottom to top and origin to center
-    bitmapDC.SetAxisOrientation( true, true );
-    bitmapDC.SetDeviceOrigin( half_size, half_size );
+    // clear background
+    wxColor bgcolor( GetBackgroundColour() );
+    wxRect rect( 0, 0, bmsize.x, bmsize.y );
+    m_bitmapHSV->SetRGB( rect, bgcolor.Red(), bgcolor.Green(), bgcolor.Blue() );
 
     // Reserve room to draw cursors inside the bitmap
     half_size -= m_cursorsSize/2;
@@ -312,16 +299,12 @@ void COLOR4D_PICKER_DLG::createHSVBitmap()
 
             color.FromHSV( hue, sat, 1.0 );
 
-            pen.SetColour( color.ToColour() );
-            bitmapDC.SetPen( pen );
-            bitmapDC.DrawPoint( xx, yy );
+            wxColor c = color.ToColour();
+            m_bitmapHSV->SetRGB( xx + half_size, -yy + half_size,
+                                 c.Red(), c.Green(), c.Blue() );
+            //m_bitmapHSV->SetAlpha( xx + half_size, -yy + half_size, 255 );
         }
     }
-
-    /* Deselect the Tool Bitmap from DC,
-     * in order to delete the MemoryDC safely without deleting the bitmap
-     */
-    bitmapDC.SelectObject( wxNullBitmap );
 }
 
 
@@ -343,7 +326,7 @@ void COLOR4D_PICKER_DLG::drawRGBPalette()
     // Reserve room to draw cursors inside the bitmap
     half_size -= m_cursorsSize/2;
 
-    // Draw the 3 RGB cursors, usiing white color to make them always visible:
+    // Draw the 3 RGB cursors, using white color to make them always visible:
     wxPen pen( wxColor( 255, 255, 255 ) );
     wxBrush brush( wxColor( 0, 0, 0 ), wxBRUSHSTYLE_TRANSPARENT );
     bitmapDC.SetPen( pen );
@@ -394,11 +377,10 @@ void COLOR4D_PICKER_DLG::drawHSVPalette()
     if( !m_bitmapHSV || m_bitmapHSV->GetSize() != m_HsvBitmap->GetSize() )
         createHSVBitmap();
 
-    wxMemoryDC bitmapDC;
     wxSize bmsize = m_bitmapHSV->GetSize();
     int half_size = std::min( bmsize.x, bmsize.y )/2;
     wxBitmap newBm( *m_bitmapHSV );
-    bitmapDC.SelectObject( newBm );
+    wxMemoryDC bitmapDC( newBm );
 
     // Use Y axis from bottom to top and origin to center
     bitmapDC.SetAxisOrientation( true, true );
@@ -411,10 +393,10 @@ void COLOR4D_PICKER_DLG::drawHSVPalette()
     m_cursorBitmapHSV.x = cos( m_hue * M_PI / 180.0 ) * half_size * m_sat;
     m_cursorBitmapHSV.y = sin( m_hue * M_PI / 180.0 ) * half_size * m_sat;
 
-    wxPen pen( wxColor( 0, 0, 0 ) );
+    wxPen pen( wxColor( 0, 0, 0 ), 2 );
     wxBrush brush( wxColor( 0, 0, 0 ), wxBRUSHSTYLE_TRANSPARENT );
-    bitmapDC.SetPen( pen );
     bitmapDC.SetBrush( brush );
+    bitmapDC.SetPen( pen );
 
     int half_csize = m_cursorsSize/2;
     bitmapDC.DrawRectangle( m_cursorBitmapHSV.x- half_csize,
diff --git a/common/widgets/color4Dpickerdlg.h b/common/widgets/color4Dpickerdlg.h
index 6c125a1..5b452c6 100644
--- a/common/widgets/color4Dpickerdlg.h
+++ b/common/widgets/color4Dpickerdlg.h
@@ -61,8 +61,8 @@ private:
     double m_sat;                       ///< the current saturation (0 ... 1.0)
     double m_val;                       ///< the current value (0 ... 1.0)
 
-    wxBitmap* m_bitmapRGB;              ///< the basic RGB palette
-    wxBitmap* m_bitmapHSV;              ///< the basic HUV palette
+    wxImage* m_bitmapRGB;              ///< the basic RGB palette
+    wxImage* m_bitmapHSV;              ///< the basic HUV palette
 
     std::vector<wxBitmapButton*> m_buttonsColor;    ///< list of defined colors buttons
 
-- 
1.9.5.msysgit.1