Use physical size in wxImageList
This seems to be the only way to fix using this class, which is fundamentally incompatible with high DPI support, with bitmaps of different logical but identical physical sizes. By using physical size we ensure that the code chopping up the provided bitmap in multiple images doesn't do it with the bitmap having the same physical size but a different logical size from all the other bitmaps. And, conceptually, logical size of the bitmaps in it really shouldn't matter to the image list as it doesn't have a scaling factor and so can't do anything useful with the logical size. Moreover, we can't add wxImageList::SetScaleFactor() neither because this class is, by design, supposed to be shareable between different controls which may use different scale factors, so it doesn't make sense to speak of the scale factor of the image list itself. This undoes the changes made in the generic implementation back inc374eefd34(Fold wxOSX-specific wxImageList into generic version, 2018-10-30) and alsoeb52e86553(Take into account scale factor of images added to wxImageList, 2021-04-03). Also update a couple of places where wxImageList is still used to use physical sizes. Unfortunately some others can't be easily fixed, e.g. global wxFileIconsTable would have to be completely rewritten. Finally, add a unit test checking that things work as expected now: previously the size of 24x24 bitmap in the image list containing (scaled) 32x32 bitmaps would be 21x21 due to sub-bitmap extraction kicking in. Closes #23994.
This commit is contained in:
Vadim Zeitlin
parent
0d4792cc2d
commit
53b4f4ddf2
9 changed files with 75 additions and 31 deletions
|
|
@ -77,6 +77,11 @@ Changes in behaviour not resulting in compilation errors
|
|||
called any longer by default, you need to explicitly enable 3.0 compatibility
|
||||
or change your code to override the newer overload, taking a wxWindow pointer.
|
||||
|
||||
- wxImageList size is now expressed in physical pixels, i.e. its size must be
|
||||
the same as the size of bitmaps added to it, in pixels. This is inconvenient
|
||||
but should be viewed as a hint not to use wxImageList (but wxBitmapBundle)
|
||||
in the applications supporting high DPI.
|
||||
|
||||
|
||||
Changes in behaviour which may result in build errors
|
||||
-----------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ public:
|
|||
virtual wxIcon GetIcon(int index) const = 0;
|
||||
|
||||
protected:
|
||||
// Size of a single bitmap in the list in logical pixels.
|
||||
// Size of a single bitmap in the list in physical pixels.
|
||||
wxSize m_size;
|
||||
|
||||
bool m_useMask = false;
|
||||
|
|
|
|||
|
|
@ -143,7 +143,7 @@ public:
|
|||
else if ( m_imageList )
|
||||
{
|
||||
// All images in the image list are of the same size.
|
||||
size = m_imageList->GetSize();
|
||||
size = window->FromPhys(m_imageList->GetSize());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -59,7 +59,8 @@ public:
|
|||
Constructor specifying the image size, whether image masks should be created,
|
||||
and the initial size of the list.
|
||||
|
||||
Note that the size is specified in logical pixels.
|
||||
Note that the size is specified in physical pixels and must correspond
|
||||
to the size of bitmaps, in pixels, that will be added to this list.
|
||||
|
||||
@param width
|
||||
Width of the images in the list.
|
||||
|
|
@ -83,10 +84,10 @@ public:
|
|||
/**
|
||||
Adds a new image or images using a bitmap and optional mask bitmap.
|
||||
|
||||
The logical size of the bitmap should be the same as the size specified
|
||||
when constructing wxImageList. If the logical width of the bitmap is
|
||||
greater than the image list width, bitmap is split into smaller images
|
||||
of the required width.
|
||||
The physical size of the bitmap should be the same as the size specified
|
||||
when constructing wxImageList. If the width of the bitmap is greater
|
||||
than the image list width, bitmap is split into smaller images of the
|
||||
required width, allowing to add multiple images from a single bitmap.
|
||||
|
||||
@param bitmap
|
||||
Bitmap representing the opaque areas of the image.
|
||||
|
|
@ -101,10 +102,10 @@ public:
|
|||
/**
|
||||
Adds a new image or images using a bitmap and mask colour.
|
||||
|
||||
The logical size of the bitmap should be the same as the size specified
|
||||
when constructing wxImageList. If the logical width of the bitmap is
|
||||
greater than the image list width, bitmap is split into smaller images
|
||||
of the required width.
|
||||
The physical size of the bitmap should be the same as the size specified
|
||||
when constructing wxImageList. If the width of the bitmap is greater
|
||||
than the image list width, bitmap is split into smaller images of the
|
||||
required width, allowing to add multiple images from a single bitmap.
|
||||
|
||||
@param bitmap
|
||||
Bitmap representing the opaque areas of the image.
|
||||
|
|
@ -118,7 +119,7 @@ public:
|
|||
/**
|
||||
Adds a new image using an icon.
|
||||
|
||||
The logical size of the icon should be the same as the size specified
|
||||
The physical size of the icon should be the same as the size specified
|
||||
when constructing wxImageList.
|
||||
|
||||
@param icon
|
||||
|
|
@ -201,9 +202,9 @@ public:
|
|||
@param index
|
||||
currently unused, should be 0
|
||||
@param width
|
||||
receives the width of the images in the list
|
||||
receives the width of the images in the list in pixels
|
||||
@param height
|
||||
receives the height of the images in the list
|
||||
receives the height of the images in the list in pixels
|
||||
|
||||
@return @true if the function succeeded, @false if it failed
|
||||
(for example, if the image list was not yet initialized).
|
||||
|
|
|
|||
|
|
@ -675,11 +675,7 @@ wxBitmapBundle::CreateImageList(wxWindow* win,
|
|||
wxCHECK_MSG( win, nullptr, "must have a valid window" );
|
||||
wxCHECK_MSG( !bundles.empty(), nullptr, "should have some images" );
|
||||
|
||||
wxSize size = GetConsensusSizeFor(win, bundles);
|
||||
|
||||
// wxImageList wants the logical size for the platforms where logical and
|
||||
// physical pixels are different.
|
||||
size /= win->GetContentScaleFactor();
|
||||
const wxSize size = GetConsensusSizeFor(win, bundles);
|
||||
|
||||
wxImageList* const iml = new wxImageList(size.x, size.y);
|
||||
|
||||
|
|
|
|||
|
|
@ -83,7 +83,7 @@ wxBitmap wxGenericImageList::GetImageListBitmap(const wxBitmap& bitmap) const
|
|||
|
||||
// Ensure image size is the same as the size of the images on the image list.
|
||||
wxBitmap bmpResized;
|
||||
const wxSize sz = bmp.GetLogicalSize();
|
||||
const wxSize sz = bmp.GetSize();
|
||||
if ( sz.x == m_size.x && sz.y == m_size.y )
|
||||
{
|
||||
bmpResized = bmp;
|
||||
|
|
@ -125,14 +125,19 @@ int wxGenericImageList::Add( const wxBitmap &bitmap )
|
|||
// Cannot add image to invalid list
|
||||
wxCHECK_MSG( m_size != wxSize(0, 0), -1, "Invalid image list" );
|
||||
|
||||
// We use the logical size here as image list images size is specified in
|
||||
// logical pixels, just as window coordinates and sizes are.
|
||||
const wxSize bitmapSize = bitmap.GetLogicalSize();
|
||||
const wxSize bitmapSize = bitmap.GetSize();
|
||||
|
||||
// There is a special case: a bitmap may contain more than one image,
|
||||
// in which case we're supposed to chop it in parts, just as Windows
|
||||
// ImageList_Add() does.
|
||||
if ( bitmapSize.x == m_size.x )
|
||||
//
|
||||
// However we don't apply this special case to bitmaps with a scale factor
|
||||
// different from 1: their actual physical size could be the same as ours
|
||||
// for all we know (we don't have a scale factor here and wxImageList API
|
||||
// prevents it from having one), so leave them alone. This is clearly a
|
||||
// hack but OTOH nobody uses multi-image bitmaps with a scale factor and it
|
||||
// avoids problems when using scaled bitmaps in the image list, see #23994.
|
||||
if ( bitmapSize.x == m_size.x || bitmap.GetScaleFactor() != 1.0 )
|
||||
{
|
||||
m_images.push_back(GetImageListBitmap(bitmap));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -800,8 +800,10 @@ void wxLogDialog::CreateDetailsControls(wxWindow *parent)
|
|||
m_listctrl->InsertColumn(1, wxT("Time"));
|
||||
|
||||
// prepare the imagelist
|
||||
static const int ICON_SIZE = 16;
|
||||
wxImageList *imageList = new wxImageList(ICON_SIZE, ICON_SIZE);
|
||||
wxSize iconSize(16, 16);
|
||||
iconSize *= parent->GetDPIScaleFactor();
|
||||
|
||||
wxImageList *imageList = new wxImageList(iconSize.x, iconSize.y);
|
||||
|
||||
// order should be the same as in the switch below!
|
||||
static wxString const icons[] =
|
||||
|
|
@ -816,7 +818,7 @@ void wxLogDialog::CreateDetailsControls(wxWindow *parent)
|
|||
for ( size_t icon = 0; icon < WXSIZEOF(icons); icon++ )
|
||||
{
|
||||
wxBitmap bmp = wxArtProvider::GetBitmap(icons[icon], wxART_MESSAGE_BOX,
|
||||
wxSize(ICON_SIZE, ICON_SIZE));
|
||||
iconSize);
|
||||
|
||||
// This may very well fail if there are insufficient colours available.
|
||||
// Degrade gracefully.
|
||||
|
|
|
|||
|
|
@ -447,16 +447,19 @@ bool wxHtmlHelpWindow::Create(wxWindow* parent, wxWindowID id,
|
|||
#endif
|
||||
);
|
||||
|
||||
wxImageList *ContentsImageList = new wxImageList(16, 16);
|
||||
wxSize iconSize(16, 16);
|
||||
iconSize *= GetDPIScaleFactor();
|
||||
|
||||
wxImageList *ContentsImageList = new wxImageList(iconSize.x, iconSize.y);
|
||||
ContentsImageList->Add(wxArtProvider::GetIcon(wxART_HELP_BOOK,
|
||||
wxART_HELP_BROWSER,
|
||||
wxSize(16, 16)));
|
||||
iconSize));
|
||||
ContentsImageList->Add(wxArtProvider::GetIcon(wxART_HELP_FOLDER,
|
||||
wxART_HELP_BROWSER,
|
||||
wxSize(16, 16)));
|
||||
iconSize));
|
||||
ContentsImageList->Add(wxArtProvider::GetIcon(wxART_HELP_PAGE,
|
||||
wxART_HELP_BROWSER,
|
||||
wxSize(16, 16)));
|
||||
iconSize));
|
||||
|
||||
m_ContentsBox->AssignImageList(ContentsImageList);
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
|
||||
#include "wx/artprov.h"
|
||||
#include "wx/dcmemory.h"
|
||||
#include "wx/imaglist.h"
|
||||
|
||||
#include "asserthelper.h"
|
||||
|
||||
|
|
@ -470,6 +471,37 @@ TEST_CASE("BitmapBundle::Scale", "[bmpbundle][scale]")
|
|||
CHECK( b.GetDefaultSize() == wxSize(8, 8) );
|
||||
}
|
||||
|
||||
TEST_CASE("BitmapBundle::ImageList", "[bmpbundle][imagelist]")
|
||||
{
|
||||
wxVector<wxBitmapBundle> images;
|
||||
images.push_back(wxBitmapBundle::FromBitmaps(wxBitmap(16, 16), wxBitmap(32, 32)));
|
||||
images.push_back(wxBitmapBundle::FromBitmap(wxBitmap(24, 24)));
|
||||
images.push_back(wxBitmapBundle::FromBitmaps(wxBitmap(16, 16), wxBitmap(32, 32)));
|
||||
|
||||
// There are 2 bundles with preferred size of 32x32, so they should win.
|
||||
const wxSize size = wxBitmapBundle::GetConsensusSizeFor(2.0, images);
|
||||
CHECK( size == wxSize(32, 32) );
|
||||
|
||||
wxImageList iml(size.x, size.y);
|
||||
for ( const auto& bundle : images )
|
||||
{
|
||||
wxBitmap bmp = bundle.GetBitmap(size);
|
||||
REQUIRE( bmp.IsOk() );
|
||||
CHECK( bmp.GetSize() == size );
|
||||
REQUIRE( iml.Add(bmp) != -1 );
|
||||
}
|
||||
|
||||
CHECK( iml.GetBitmap(0).GetSize() == size );
|
||||
#ifdef wxHAS_DPI_INDEPENDENT_PIXELS
|
||||
CHECK( iml.GetBitmap(0).GetScaleFactor() == 2 );
|
||||
#endif
|
||||
|
||||
CHECK( iml.GetBitmap(1).GetSize() == size );
|
||||
#ifdef wxHAS_DPI_INDEPENDENT_PIXELS
|
||||
CHECK( iml.GetBitmap(1).GetScaleFactor() == Approx(1.3333333333) );
|
||||
#endif
|
||||
}
|
||||
|
||||
#endif // ports with scaled bitmaps support
|
||||
|
||||
TEST_CASE("BitmapBundle::GetConsensusSize", "[bmpbundle]")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue