Merge branch 'tbar-bitmap-size'

Improve handling toolbar tools bitmap size.

See #22488.
This commit is contained in:
Vadim Zeitlin 2022-06-05 14:43:29 +02:00
commit 4b83ed83ec
7 changed files with 84 additions and 62 deletions

View File

@ -299,7 +299,8 @@ programs involves doing at least the following:
wxArtProvider::CreateBitmapBundle() (and, of course, return multiple bitmaps
from it) instead of wxArtProvider::CreateBitmap().
4. Removing any calls to wxToolBar::SetToolBitmapSize() or, equivalently,
`<bitmapsize>` attributes from the XRC, as this forces unwanted scaling.
`<bitmapsize>` attributes from the XRC, as this overrides the best size
determination and may result in suboptimal appearance.

View File

@ -152,11 +152,11 @@ public:
// Implementation only from now on.
// Get the bitmap size preferred by the majority of the elements of the
// bundles at the scale appropriate for the given scale.
// bundles at the given scale or the scale appropriate for the given window.
static wxSize
GetConsensusSizeFor(wxWindow* win,
const wxVector<wxBitmapBundle>& bundles,
const wxSize& sizeDefault);
GetConsensusSizeFor(double scale, const wxVector<wxBitmapBundle>& bundles);
static wxSize
GetConsensusSizeFor(wxWindow* win, const wxVector<wxBitmapBundle>& bundles);
// Create wxImageList and fill it with the images from the given bundles in
// the sizes appropriate for the DPI scaling used for the specified window.

View File

@ -882,15 +882,15 @@ public:
It is usually unnecessary to call this function, as the tools will
always be made big enough to fit the size of the bitmaps used in them.
Moreover, calling it may force wxToolBar to scale its images, even
using non-integer scaling factor, which will usually look bad, instead
of adapting the image size to the current DPI scaling in order to avoid
doing this.
Moreover, calling it forces wxToolBar to scale its images in high DPI
using the provided size, instead of letting wxBitmapBundle used for the
tool bitmaps determine the best suitable bitmap size, which may result
in suboptimal appearance.
If you do call it, it must be done before toolbar is Realize()'d.
Example of using this function to force the bitmaps to be at least
32 pixels wide and tall:
32 pixels wide and tall (at normal DPI):
@code
toolbar->SetToolBitmapSize(FromDIP(wxSize(32, 32)));
toolbar->AddTool(wxID_NEW, "New", wxBitmapBundle::FromXXX(...));
@ -898,9 +898,6 @@ public:
toolbar->Realize();
@endcode
Note that this example would scale bitmaps to 48 pixels when using 150%
DPI scaling, which wouldn't happen without calling SetToolBitmapSize().
@param size
The size of the bitmaps in the toolbar in logical pixels.

View File

@ -593,12 +593,16 @@ void RecordSizePref(SizePrefs& prefs, const wxSize& size)
/* static */
wxSize
wxBitmapBundle::GetConsensusSizeFor(wxWindow* win,
const wxVector<wxBitmapBundle>& bundles,
const wxSize& sizeDefault)
const wxVector<wxBitmapBundle>& bundles)
{
const double scale = win->GetDPIScaleFactor();
const wxSize sizeIdeal = sizeDefault*scale;
return GetConsensusSizeFor(win->GetDPIScaleFactor(), bundles);
}
/* static */
wxSize
wxBitmapBundle::GetConsensusSizeFor(double scale,
const wxVector<wxBitmapBundle>& bundles)
{
// We want to use preferred bitmap size, but the preferred sizes can be
// different for different bitmap bundles, so record all their preferences
// first.
@ -623,25 +627,13 @@ wxBitmapBundle::GetConsensusSizeFor(wxWindow* win,
}
else if ( countThis == countMax )
{
// We have a tie between different sizes, choose the one
// corresponding to the current scale factor, if possible, as this
// is the ideal bitmap size that should be consistent with all the
// other bitmaps.
if ( sizePreferred != sizeIdeal )
{
if ( sizeThis == sizeIdeal )
{
sizePreferred = sizeThis;
}
else // Neither of the sizes is the ideal one.
{
// Choose the larger one as like this some bitmaps will be
// downscaled, which should look better than upscaling some
// (other) ones.
if ( sizeThis.y > sizePreferred.y )
sizePreferred = sizeThis;
}
}
// We have a tie between different sizes.
// Choose the larger one as like this some bitmaps will be
// downscaled, which should look better than upscaling some
// (other) ones.
if ( sizeThis.y > sizePreferred.y )
sizePreferred = sizeThis;
}
}
@ -656,12 +648,7 @@ wxBitmapBundle::CreateImageList(wxWindow* win,
wxCHECK_MSG( win, NULL, "must have a valid window" );
wxCHECK_MSG( !bundles.empty(), NULL, "should have some images" );
// We arbitrarily choose the default size of the first bundle as the
// default size for the image list too, as it's not clear what else could
// we do here. Note that this size is only used to break the tie in case
// the same number of bundles prefer two different sizes, so it's not going
// to matter at all in most cases.
wxSize size = GetConsensusSizeFor(win, bundles, bundles[0].GetDefaultSize());
wxSize size = GetConsensusSizeFor(win, bundles);
// wxImageList wants the logical size for the platforms where logical and
// physical pixels are different.

View File

@ -72,8 +72,7 @@ void wxBitmapComboBoxBase::UpdateInternals()
m_usedImgSize = wxBitmapBundle::GetConsensusSizeFor
(
GetControl(),
m_bitmapbundles,
wxSize(0, 0)
m_bitmapbundles
);
}
}

View File

@ -475,14 +475,26 @@ void wxToolBarBase::AdjustToolBitmapSize()
bundles.push_back(bmp);
}
if ( !bundles.empty() )
if ( bundles.empty() )
return;
wxSize sizeNeeded;
if ( m_requestedBitmapSize != wxSize(0, 0) )
{
wxSize sizePreferred = wxBitmapBundle::GetConsensusSizeFor
(
this,
bundles,
ToDIP(sizeOrig)
);
// If we have a fixed requested bitmap size, use it, but scale it by
// integer factor only, as otherwise we'd force fractional (and hence
// ugly looking) scaling here whenever fractional DPI scaling is used.
// We want to round 1.5 down to 1, but 1.75 up to 2.
int scaleFactorRoundedDown =
static_cast<int>(ceil(2*GetDPIScaleFactor())) / 2;
sizeNeeded = m_requestedBitmapSize*scaleFactorRoundedDown;
}
else // Determine the best size to use from the bitmaps we have.
{
const wxSize
sizePreferred = wxBitmapBundle::GetConsensusSizeFor(this, bundles);
// GetConsensusSizeFor() returns physical size, but we want to operate
// with logical pixels as everything else is expressed in them.
@ -492,22 +504,17 @@ void wxToolBarBase::AdjustToolBitmapSize()
// scale factor may be different from 1) use this size at all
// currently, so it shouldn't matter. But if/when they are modified to
// use the size computed here, this would need to be revisited.
sizePreferred = FromPhys(sizePreferred);
// Don't decrease the bitmap below the size requested by the application
// as using larger bitmaps shouldn't shrink them to the small default
// size.
sizePreferred.IncTo(FromDIP(m_requestedBitmapSize));
// No need to change the bitmaps size if it doesn't really change.
if ( sizePreferred == sizeOrig )
return;
sizeNeeded = FromPhys(sizePreferred);
}
// No need to change the bitmaps size if it doesn't really change.
if ( sizeNeeded != sizeOrig )
{
// Call DoSetToolBitmapSize() and not SetToolBitmapSize() to avoid
// changing the requested bitmap size: if we set our own adjusted size
// as the preferred one, we wouldn't decrease it later even if we ought
// to, as when moving from a monitor with higher DPI to a lower-DPI one.
DoSetToolBitmapSize(sizePreferred);
DoSetToolBitmapSize(sizeNeeded);
}
}

View File

@ -460,3 +460,34 @@ TEST_CASE("BitmapBundle::Scale", "[bmpbundle][scale]")
}
#endif // ports with scaled bitmaps support
TEST_CASE("BitmapBundle::GetConsensusSize", "[bmpbundle]")
{
// Just a trivial helper to make writing the tests below simpler.
struct Bundles
{
wxVector<wxBitmapBundle> vec;
void Add(int size)
{
vec.push_back(wxBitmapBundle::FromBitmap(wxSize(size, size)));
}
int GetConsensusSize(double scale) const
{
return wxBitmapBundle::GetConsensusSizeFor(scale, vec).y;
}
} bundles;
// When there is a tie, a larger size is chosen by default.
bundles.Add(16);
bundles.Add(24);
CHECK( bundles.GetConsensusSize(2) == 48 );
// Breaking the tie results in the smaller size winning now.
bundles.Add(16);
CHECK( bundles.GetConsensusSize(2) == 32 );
// Integer scaling factors should be preferred.
CHECK( bundles.GetConsensusSize(1.5) == 16 );
}