Choice changes for wxTextMeasure

[mmarsan]

This patch starts the use of wxTextMeasure::GetLargestStringExtent()
It requires the patch in ticket 14705.

Changes are in GetBestSize() of src/msw/choice.cpp and src/gtk/choice.cpp

This code can be also easily copied to other (i.e. OX) choice and combobox implementations.

[mmarsan]

(In #14705) I attach the version 2 (replaces the other)

[mmarsan]

I attach the new version (v2) reworked for v2 of patch in ticket 14705

[mmarsan]

Replaces the other

[vadz]

(In #14705) Just FYI: thanks for the update, I'm working on applying this.

[VZ]

(In #14705) (In [72699]) Factor out text measurement from wxDC and wxWindow into wxTextMeasure.

Add a new private wxTextMeasure class implementing methods for measuring text
and move the often duplicated (but not always identically) code for doing the
same from wxDC and wxWindow into it.

Currently this class is only really implemented in wxMSW and wxGTK.

Also extend the test for text measuring functions and rename it to
MeasuringTextTestCase from MeasuringContextTestCase as it's not wxGC-specific
any more.

Closes #14705.

[vadz]

The code is so similar in the 2 ports that it would be really better to put it into wxChoiceBase. You could use #if !wxUSE_GENERIC_TEXTMEASURE to do it only for the ports that actually have wxTextMeasure or just do it unconditionally, AFAICS it shouldn't do any lasting harm neither.

Could you please update the patch to do it at the base class level? TIA!

[mmarsan]

Gather code in wxChoiceBase instead of msw/gtk wxChoice versions

[mmarsan]

I've attached the version 3, replaces the others.

[vadz]

Thanks, but it's still unfortunately not perfect. Here is what I'd like to see improved:

1. Provide wxChoiceBase::DoGetBestSize() for all ports. As wxTextMeasure can now be used everywhere (by just forwarding back to wxDC if not implemented natively), I don't see any reason to explicitly test for __WXMSW__ || __WXGTK__ here (and if such reason does exist, it would still be better to use #if !wxUSE_GENERIC_TEXTMEASURE instead of explicitly hardcoding the platforms currently implementing it).
2. Remove CacheBestSize() from there, it's already done by wxWindowBasE::GetBestSize().
3. Important: avoid platform-specific code in wxChoiceBase. The whole idea of having it is to avoid these ugly #ifdefs. Instead, the base class version should contain only the code really common to all platforms and wxMSW should still continue to override it and handle wxCB_SIMPLE by adding the extra margin there (as for wxGTK, I don't think it's special case is needed at all but I could be wrong).

Could you please update the patch like this? TIA!

[mmarsan]

#if !wxUSE_GENERIC_TEXTMEASURE may not be yet defined for the time wxChoiceBase gets compiled: But __WXMSW__ __WXGTK__ are.

[mmarsan]

Replying to mmarsan:

#if !wxUSE_GENERIC_TEXTMEASURE may not be yet defined for the time wxChoiceBase gets compiled: But __WXMSW__ __WXGTK__ are.

Forget this comment. wx/private/textmeasure.h is included so wxUSE_GENERIC_TEXTMEASURE is defined.

[mmarsan]

4th version. I hope it is the last :)

[vadz]

Thanks, much better but I don't really understand why do we need to test for wxUSE_GENERIC_TEXTMEASURE, wxTextMeasure is supposed to work in this case too, so I've just removed it. Please let me know if I broke something.

I also don't see any need for the code dealing with wxFont explicitly, using the window font should be the default behaviour anyhow and if it isn't, then it's a problem in wxTextMeasure itself and should be fixed there.

[vadz]

For the reference, here is the code I used for testing:

samples/minimal/minimal.cpp

@@ -172 +172 @@
     CreateStatusBar(2);
     SetStatusText("Welcome to wxWidgets!");
 #endif // wxUSE_STATUSBAR
+
+    wxString choices[] = { "first", "very very very very long", "last" };
+    wxSizer* sizer = new wxBoxSizer(wxVERTICAL);
+
+    wxChoice* c1 = new wxChoice(this, wxID_ANY);
+    sizer->Add(c1);
+
+    wxChoice* c2 = new wxChoice(this, wxID_ANY);
+    c2->Append(WXSIZEOF(choices), choices);
+    sizer->Add(c2);
+
+    wxChoice* c3 = new wxChoice(this, wxID_ANY);
+    c3->SetFont(wxNORMAL_FONT->Scaled(2));
+    c3->Append(WXSIZEOF(choices), choices);
+    sizer->Add(c3);
+
+    SetSizerAndFit(sizer);
 }
 
 

[vadz]

Ugh, testing under MSW shows that the best size is broken, the last choice above is now truncated... Looking into it...

[VZ]

(In [72844]) Allow constructing wxGTK wxTextMeasure with NULL font.

The font is explicitly documented as being possibly NULL in the base class and
wxMSW handles NULL font just fine, so also handle it in the GTK version.

See #14706.

[VZ]

(In [72848]) Refactor and simplify wxChoice::DoGetBestSize().

Use wxTextMeasure instead of duplicating its code and also reuse the code
between different ports.

Closes #14706.