From a02b5f2f1fc00cd53059f2ff2a8bc1d37b96224e Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 19 Jan 2026 15:13:14 -0800 Subject: [PATCH] Fix image rotation direction in BITMAP_BASE::Rotate The wxImage::Rotate90() parameter means "clockwise" when true, but the code was passing aRotateCCW directly. This caused the pixel rotation to be opposite to the intended direction. The bug manifested as images appearing incorrectly after save/reload, since the OpenGL renderer uses m_originalImage with m_rotation compensation, while after reload the already-rotated pixels have no rotation compensation. Also updated the unit test that was written to match the buggy behavior, and added a separate test case for CW rotation. Fixes https://gitlab.com/kicad/code/kicad/-/issues/22719 --- common/bitmap_base.cpp | 3 ++- qa/tests/common/test_bitmap_base.cpp | 39 +++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/common/bitmap_base.cpp b/common/bitmap_base.cpp index 0e5d4cda1b..9368c5e1c8 100644 --- a/common/bitmap_base.cpp +++ b/common/bitmap_base.cpp @@ -450,7 +450,8 @@ void BITMAP_BASE::Rotate( bool aRotateCCW ) int resY = m_image->GetOptionInt( wxIMAGE_OPTION_RESOLUTIONY ); int unit = m_image->GetOptionInt( wxIMAGE_OPTION_RESOLUTIONUNIT ); - *m_image = m_image->Rotate90( aRotateCCW ); + // wxImage::Rotate90 parameter is "clockwise", so invert for CCW rotation + *m_image = m_image->Rotate90( !aRotateCCW ); m_image->SetOption( wxIMAGE_OPTION_RESOLUTIONUNIT, unit ); m_image->SetOption( wxIMAGE_OPTION_RESOLUTIONX, resX ); diff --git a/qa/tests/common/test_bitmap_base.cpp b/qa/tests/common/test_bitmap_base.cpp index d5471bf2b0..a06c1a72d5 100644 --- a/qa/tests/common/test_bitmap_base.cpp +++ b/qa/tests/common/test_bitmap_base.cpp @@ -192,18 +192,18 @@ BOOST_AUTO_TEST_CASE( BasicImage ) } /** - * Check the image is right after rotating + * Check the image is right after rotating CCW */ -BOOST_AUTO_TEST_CASE( RotateImage ) +BOOST_AUTO_TEST_CASE( RotateImageCCW ) { - // Note the parameter name is wrong here, true is clockwise - m_4tile.Rotate( false ); + m_4tile.Rotate( true ); // true = CCW const wxImage* img_data = m_4tile.GetImageData(); BOOST_REQUIRE_NE( img_data, nullptr ); - // black, blue, - // green, red + // Original: After 90 CCW: + // green, black, black, blue, + // red, blue green, red const std::vector exp_pixels = { { 1, 1, col_black }, { 6, 1, col_blue }, @@ -218,6 +218,33 @@ BOOST_AUTO_TEST_CASE( RotateImage ) } } +/** + * Check the image is right after rotating CW + */ +BOOST_AUTO_TEST_CASE( RotateImageCW ) +{ + m_4tile.Rotate( false ); // false = CW + + const wxImage* img_data = m_4tile.GetImageData(); + BOOST_REQUIRE_NE( img_data, nullptr ); + + // Original: After 90 CW: + // green, black, red, green, + // red, blue blue, black + const std::vector exp_pixels = { + { 1, 1, col_red }, + { 6, 1, col_green }, + { 1, 6, col_blue }, + { 6, 6, col_black }, + }; + + for( const auto& c : exp_pixels ) + { + BOOST_CHECK_PREDICATE( + KI_TEST::IsImagePixelOfColor, ( *img_data )( c.m_x )( c.m_y )( c.m_color ) ); + } +} + /** * Check the image is right after mirroring */