diff --git a/common/bitmap_base.cpp b/common/bitmap_base.cpp index 319824d04b..cf3a22c11e 100644 --- a/common/bitmap_base.cpp +++ b/common/bitmap_base.cpp @@ -239,7 +239,7 @@ bool BITMAP_BASE::LoadLegacyData( LINE_READER& aLine, wxString& aErrorMsg ) m_image->LoadFile( istream, wxBITMAP_TYPE_ANY ); m_bitmap = new wxBitmap( *m_image ); m_originalImage = new wxImage( *m_image ); - UpdateImageDataBuffer(); + updateImageDataBuffer(); break; } @@ -433,7 +433,7 @@ void BITMAP_BASE::Mirror( FLIP_DIRECTION aFlipDirection ) m_isMirroredX = !m_isMirroredX; rebuildBitmap( false ); - UpdateImageDataBuffer(); + updateImageDataBuffer(); } } @@ -458,7 +458,7 @@ void BITMAP_BASE::Rotate( bool aRotateCCW ) m_rotation += ( aRotateCCW ? ANGLE_90 : -ANGLE_90 ); rebuildBitmap( false ); - UpdateImageDataBuffer(); + updateImageDataBuffer(); } } @@ -470,7 +470,7 @@ void BITMAP_BASE::ConvertToGreyscale() *m_image = m_image->ConvertToGreyscale(); *m_originalImage = m_originalImage->ConvertToGreyscale(); rebuildBitmap(); - UpdateImageDataBuffer(); + updateImageDataBuffer(); } } @@ -490,7 +490,7 @@ void BITMAP_BASE::PlotImage( PLOTTER* aPlotter, const VECTOR2I& aPos, } -void BITMAP_BASE::UpdateImageDataBuffer() +void BITMAP_BASE::updateImageDataBuffer() { if( m_image ) { diff --git a/common/drawing_sheet/ds_data_model_io.cpp b/common/drawing_sheet/ds_data_model_io.cpp index 7dfd19b523..e3dc1d086f 100644 --- a/common/drawing_sheet/ds_data_model_io.cpp +++ b/common/drawing_sheet/ds_data_model_io.cpp @@ -34,10 +34,11 @@ #include <drawing_sheet/drawing_sheet_lexer.h> #include <drawing_sheet/ds_file_versions.h> #include <font/font.h> +#include <io/kicad/kicad_io_utils.h> #include <string_utils.h> -#include <wx/base64.h> #include <wx/msgdlg.h> +#include <wx/mstream.h> using namespace DRAWINGSHEET_T; @@ -422,21 +423,11 @@ void DS_DATA_MODEL_IO::format( DS_DATA_ITEM_BITMAP* aItem ) const // Write image in png readable format m_out->Print( "(data" ); - wxString out = wxBase64Encode( aItem->m_ImageBitmap->GetImageDataBuffer() ); + wxMemoryOutputStream stream; + aItem->m_ImageBitmap->SaveImageData( stream ); - // Apparently the MIME standard character width for base64 encoding is 76 (unconfirmed) - // so use it in a vain attempt to be standard like. -#define MIME_BASE64_LENGTH 76 + KICAD_FORMAT::FormatStreamData( *m_out, *stream.GetOutputStreamBuffer() ); - size_t first = 0; - - while( first < out.Length() ) - { - m_out->Print( "\n\"%s\"", TO_UTF8( out( first, MIME_BASE64_LENGTH ) ) ); - first += MIME_BASE64_LENGTH; - } - - m_out->Print( ")" ); // Closes data token. m_out->Print( ")" ); // Closes bitmap token. } diff --git a/common/io/kicad/kicad_io_utils.cpp b/common/io/kicad/kicad_io_utils.cpp index b9474c7669..4dd5ff083a 100644 --- a/common/io/kicad/kicad_io_utils.cpp +++ b/common/io/kicad/kicad_io_utils.cpp @@ -17,10 +17,16 @@ * with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include "io/kicad/kicad_io_utils.h" + +// For some reason wxWidgets is built with wxUSE_BASE64 unset so expose the wxWidgets +// base64 code. +#define wxUSE_BASE64 1 +#include <wx/base64.h> + #include <fmt/format.h> #include <kiid.h> -#include <io/kicad/kicad_io_utils.h> #include <richio.h> #include <string_utils.h> @@ -37,6 +43,29 @@ void FormatUuid( OUTPUTFORMATTER* aOut, const KIID& aUuid ) aOut->Print( "(uuid %s)", aOut->Quotew( aUuid.AsString() ).c_str() ); } + +void FormatStreamData( OUTPUTFORMATTER& aOut, const wxStreamBuffer& aStream ) +{ + aOut.Print( "(data" ); + + const wxString out = wxBase64Encode( aStream.GetBufferStart(), aStream.GetBufferSize() ); + + // Apparently the MIME standard character width for base64 encoding is 76 (unconfirmed) + // so use it in a vein attempt to be standard like. + static constexpr unsigned MIME_BASE64_LENGTH = 76; + + size_t first = 0; + + while( first < out.Length() ) + { + aOut.Print( "\n\"%s\"", TO_UTF8( out( first, MIME_BASE64_LENGTH ) ) ); + first += MIME_BASE64_LENGTH; + } + + aOut.Print( ")" ); // Closes data token. +} + + /* * Formatting rules: * - All extra (non-indentation) whitespace is trimmed diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp index 85a2e0d9a4..30b04a28be 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp @@ -22,10 +22,6 @@ #include <algorithm> -// For some reason wxWidgets is built with wxUSE_BASE64 unset so expose the wxWidgets -// base64 code. -#define wxUSE_BASE64 1 -#include <wx/base64.h> #include <wx/log.h> #include <wx/mstream.h> @@ -958,23 +954,11 @@ void SCH_IO_KICAD_SEXPR::saveBitmap( const SCH_BITMAP& aBitmap ) KICAD_FORMAT::FormatUuid( m_out, aBitmap.m_Uuid ); - m_out->Print( "(data" ); + wxMemoryOutputStream stream; + bitmapBase.SaveImageData( stream ); - wxString out = wxBase64Encode( bitmapBase.GetImageDataBuffer() ); + KICAD_FORMAT::FormatStreamData( *m_out, *stream.GetOutputStreamBuffer() ); - // Apparently the MIME standard character width for base64 encoding is 76 (unconfirmed) - // so use it in a vein attempt to be standard like. -#define MIME_BASE64_LENGTH 76 - - size_t first = 0; - - while( first < out.Length() ) - { - m_out->Print( "\n\"%s\"", TO_UTF8( out( first, MIME_BASE64_LENGTH ) ) ); - first += MIME_BASE64_LENGTH; - } - - m_out->Print( ")" ); // Closes data token. m_out->Print( ")" ); // Closes image token. } diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index df0bbf8347..3e2fdcd0d1 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -1657,7 +1657,14 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) { // Just image data auto bitmap = std::make_unique<SCH_BITMAP>(); - bitmap->GetReferenceImage().SetImage( *clipImg ); + + bool ok = bitmap->GetReferenceImage().SetImage( *clipImg ); + + if( !ok ) + { + delete tempScreen; + return 0; + } tempScreen->Append( bitmap.release() ); } diff --git a/include/bitmap_base.h b/include/bitmap_base.h index 366e4f22a4..654989ad19 100644 --- a/include/bitmap_base.h +++ b/include/bitmap_base.h @@ -240,17 +240,12 @@ public: */ void SetImageType( wxBitmapType aType ) { m_imageType = aType; } - /** - * @return the image data buffer. - */ - const wxMemoryBuffer& GetImageDataBuffer() const { return m_imageData; } - +private: /** * Resets the image data buffer using the current image data. */ - void UpdateImageDataBuffer(); + void updateImageDataBuffer(); -private: /** * Rebuild the internal bitmap used to draw/plot image. * diff --git a/include/io/kicad/kicad_io_utils.h b/include/io/kicad/kicad_io_utils.h index 54740cc093..ee550b8c20 100644 --- a/include/io/kicad/kicad_io_utils.h +++ b/include/io/kicad/kicad_io_utils.h @@ -17,11 +17,13 @@ * with this program. If not, see <http://www.gnu.org/licenses/>. */ -#ifndef KICAD_IO_UTILS_H -#define KICAD_IO_UTILS_H +#pragma once +#include <wx/stream.h> #include <wx/string.h> +#include <kicommon.h> + class OUTPUTFORMATTER; class KIID; @@ -38,8 +40,11 @@ KICOMMON_API void FormatBool( OUTPUTFORMATTER* aOut, const wxString& aKey, bool KICOMMON_API void FormatUuid( OUTPUTFORMATTER* aOut, const KIID& aUuid ); +/** + * Write binary data to the formatter as base 64 encoded string. + */ +KICOMMON_API void FormatStreamData( OUTPUTFORMATTER& aOut, const wxStreamBuffer& aStream ); + KICOMMON_API void Prettify( std::string& aSource, bool aCompactSave ); } // namespace KICAD_FORMAT - -#endif //KICAD_IO_UTILS_H diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp index e727dc19f9..cc7a19d76a 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp @@ -22,9 +22,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -// base64 code. Needed for PCB_REFERENCE_IMAGE -#define wxUSE_BASE64 1 -#include <wx/base64.h> #include <wx/dir.h> #include <wx/ffile.h> #include <wx/log.h> @@ -1066,23 +1063,10 @@ void PCB_IO_KICAD_SEXPR::format( const PCB_REFERENCE_IMAGE* aBitmap ) const if( aBitmap->IsLocked() ) KICAD_FORMAT::FormatBool( m_out, "locked", true ); - m_out->Print( "(data" ); + wxMemoryOutputStream ostream; + refImage.GetImage().SaveImageData( ostream ); - wxString out = wxBase64Encode( refImage.GetImage().GetImageDataBuffer() ); - - // Apparently the MIME standard character width for base64 encoding is 76 (unconfirmed) - // so use it in a vain attempt to be standard like. -#define MIME_BASE64_LENGTH 76 - - size_t first = 0; - - while( first < out.Length() ) - { - m_out->Print( "\n\"%s\"", TO_UTF8( out( first, MIME_BASE64_LENGTH ) ) ); - first += MIME_BASE64_LENGTH; - } - - m_out->Print( ")" ); // Closes data token. + KICAD_FORMAT::FormatStreamData( *m_out, *ostream.GetOutputStreamBuffer() ); KICAD_FORMAT::FormatUuid( m_out, aBitmap->m_Uuid ); m_out->Print( ")" ); // Closes image token. diff --git a/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h b/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h index 40d2a4a8e0..fbb1b1bf97 100644 --- a/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h +++ b/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h @@ -142,6 +142,7 @@ std::ostream& boost_test_print_type( std::ostream& os, std::pair<K, V> const& aP */ std::ostream& boost_test_print_type( std::ostream& os, wxPoint const& aVec ); +std::ostream& boost_test_print_type( std::ostream& os, wxSize const& aSize ); namespace KI_TEST { diff --git a/qa/qa_utils/wx_utils/unit_test_utils.cpp b/qa/qa_utils/wx_utils/unit_test_utils.cpp index f7eee00e94..6ad4ba31c4 100644 --- a/qa/qa_utils/wx_utils/unit_test_utils.cpp +++ b/qa/qa_utils/wx_utils/unit_test_utils.cpp @@ -31,6 +31,13 @@ std::ostream& boost_test_print_type( std::ostream& os, wxPoint const& aPt ) } +std::ostream& boost_test_print_type( std::ostream& os, wxSize const& aSize ) +{ + os << "wxSize[ x=\"" << aSize.x << "\" y=\"" << aSize.y << "\" ]"; + return os; +} + + std::string KI_TEST::GetEeschemaTestDataDir() { const char* env = std::getenv( "KICAD_TEST_EESCHEMA_DATA_DIR" ); diff --git a/qa/tests/common/test_bitmap_base.cpp b/qa/tests/common/test_bitmap_base.cpp index 95834c60ab..d5471bf2b0 100644 --- a/qa/tests/common/test_bitmap_base.cpp +++ b/qa/tests/common/test_bitmap_base.cpp @@ -244,4 +244,37 @@ BOOST_AUTO_TEST_CASE( MirrorImage ) } } +/** + * Check setting image data by SetImage produces saveable data + * via SaveImageData. + * + * Regression test for: https://gitlab.com/kicad/code/kicad/-/issues/19772 + */ +BOOST_AUTO_TEST_CASE( SetImageOutputsData ) +{ + BITMAP_BASE bitmap; + wxImage img( 2, 2 ); + img.SetRGB( 0, 0, 0, 255, 0 ); + img.SetRGB( 1, 0, 0, 0, 255 ); + img.SetRGB( 0, 1, 255, 0, 0 ); + img.SetRGB( 1, 1, 0, 0, 255 ); + + // Set the wxImage directly, not via a stream/file + // (this happens, e.g. when the clipboard gives you a wxImage) + bitmap.SetImage( img ); + + wxMemoryOutputStream mos; + BOOST_CHECK( bitmap.SaveImageData( mos ) ); + + BOOST_REQUIRE( mos.GetSize() > 0 ); + + // check the output is the same as the input + wxMemoryInputStream mis( mos ); + wxImage img2; + + BOOST_CHECK( img2.LoadFile( mis, wxBITMAP_TYPE_PNG ) ); + + BOOST_CHECK_PREDICATE( KI_TEST::ImagesHaveSamePixels, (img) ( img2 ) ); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/qa/tests/common/wximage_test_utils.cpp b/qa/tests/common/wximage_test_utils.cpp index ce60c73606..9da1108632 100644 --- a/qa/tests/common/wximage_test_utils.cpp +++ b/qa/tests/common/wximage_test_utils.cpp @@ -28,15 +28,7 @@ namespace KI_TEST { -/** - * Predicate to check an image pixel matches color and alpha - * - * @param aImage the image to check - * @param aX pixel x-coordinate - * @param aY pixel y-coordinate - * @param aColor expected color (alpha is 1.0 if image doesn't support alpha) - * @return true if colour match - */ + bool IsImagePixelOfColor( const wxImage& aImage, int aX, int aY, const KIGFX::COLOR4D& aColor ) { const wxSize imageSize = aImage.GetSize(); @@ -64,6 +56,40 @@ bool IsImagePixelOfColor( const wxImage& aImage, int aX, int aY, const KIGFX::CO return true; } + +bool ImagesHaveSamePixels( const wxImage& aImgA, const wxImage& aImgB ) +{ + if( aImgA.GetSize() != aImgB.GetSize() ) + { + BOOST_TEST_INFO( "Image sizes differ: " << aImgA.GetSize() << " vs " << aImgB.GetSize() ); + return false; + } + + for( int y = 0; y < aImgA.GetHeight(); ++y ) + { + for( int x = 0; x < aImgA.GetWidth(); ++x ) + { + const int rA = aImgA.GetRed( x, y ); + const int gA = aImgA.GetGreen( x, y ); + const int bA = aImgA.GetBlue( x, y ); + + const int rB = aImgB.GetRed( x, y ); + const int gB = aImgB.GetGreen( x, y ); + const int bB = aImgB.GetBlue( x, y ); + + if( rA != rB || gA != gB || bA != bB ) + { + BOOST_TEST_INFO( "Pixel (" << x << ", " << y << ") differs: " + << "A(" << rA << ", " << gA << ", " << bA << ") " + << "B(" << rB << ", " << gB << ", " << bB << ")" ); + return false; + } + } + } + + return true; +} + } // namespace KI_TEST diff --git a/qa/tests/common/wximage_test_utils.h b/qa/tests/common/wximage_test_utils.h index 95be8e515b..de34134b7f 100644 --- a/qa/tests/common/wximage_test_utils.h +++ b/qa/tests/common/wximage_test_utils.h @@ -43,6 +43,13 @@ namespace KI_TEST */ bool IsImagePixelOfColor( const wxImage& aImage, int aX, int aY, const KIGFX::COLOR4D& aColor ); +/** + * Check if an image is identical to another image, pixel by pixel + * + * (this is exhaustive, avoid using it for large images) + */ +bool ImagesHaveSamePixels( const wxImage& aImgA, const wxImage& aImgB ); + } // namespace KI_TEST