#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  normal                   |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:        
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -
 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.

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  normal                   |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -
Changes (by vadz):

  * blockedby:  => 14705

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:              
     Type:  optimization             |      Status:  new        
 Priority:  normal                   |   Milestone:              
Component:  GUI-all                  |     Version:  2.9-svn    
 Keywords:  choice combo textextent  |   Blockedby:  14705, 14705
    Patch:  1                        |    Blocking:              
-------------------------------------+------------------------------------- -

Comment(by mmarsan):

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

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -
Changes (by vadz):

  * priority:  normal => low
  * blockedby:  14705, 14705, 14705, 14705 => 14705

Comment:

 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!

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -

Comment(by mmarsan):

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

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -

Comment(by 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).
  1. Remove `CacheBestSize()` from there, it's already done by
 `wxWindowBasE::GetBestSize()`.
  1. 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!

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -

Comment(by mmarsan):

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

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -

Comment(by mmarsan):

 Replying to [comment:8 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.

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:        
     Type:  optimization             |      Status:  new    
 Priority:  low                      |   Milestone:        
Component:  GUI-all                  |     Version:  2.9-svn
 Keywords:  choice combo textextent  |   Blockedby:  14705  
    Patch:  1                        |    Blocking:        
-------------------------------------+------------------------------------- -

Comment(by mmarsan):

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

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:          
     Type:  optimization             |      Status:  confirmed
 Priority:  low                      |   Milestone:          
Component:  GUI-all                  |     Version:  2.9-svn  
 Keywords:  choice combo textextent  |   Blockedby:  14705    
    Patch:  1                        |    Blocking:          
-------------------------------------+------------------------------------- -
Changes (by vadz):

  * status:  new => confirmed

Comment:

 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.

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:          
     Type:  optimization             |      Status:  confirmed
 Priority:  low                      |   Milestone:          
Component:  GUI-all                  |     Version:  2.9-svn  
 Keywords:  choice combo textextent  |   Blockedby:  14705    
    Patch:  1                        |    Blocking:          
-------------------------------------+------------------------------------- -

Comment(by vadz):

 For the reference, here is the code I used for testing:
 {{{
 #!diff
 diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
 index a78e462..89b4682 100644
 --- a/samples/minimal/minimal.cpp
 +++ b/samples/minimal/minimal.cpp
 @@ -172,6 +172,23 @@ bool MyApp::OnInit()
      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);
  }

 }}}

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:          
     Type:  optimization             |      Status:  confirmed
 Priority:  low                      |   Milestone:          
Component:  GUI-all                  |     Version:  2.9-svn  
 Keywords:  choice combo textextent  |   Blockedby:  14705    
    Patch:  1                        |    Blocking:          
-------------------------------------+------------------------------------- -

Comment(by vadz):

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

#14706: Choice changes for wxTextMeasure
-------------------------------------+------------------------------------- -
 Reporter:  mmarsan                  |       Owner:          
     Type:  optimization             |      Status:  confirmed
 Priority:  low                      |   Milestone:          
Component:  GUI-all                  |     Version:  2.9-svn  
 Keywords:  choice combo textextent  |   Blockedby:  14705    
    Patch:  1                        |    Blocking:          
-------------------------------------+------------------------------------- -

Comment(by 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.

#14706: Choice changes for wxTextMeasure
---------------------------+----------------------------------------------- -
  Reporter:  mmarsan       |       Owner:                        
      Type:  optimization  |      Status:  closed                
  Priority:  low           |   Milestone:                        
 Component:  GUI-all       |     Version:  2.9-svn                
Resolution:  fixed         |    Keywords:  choice combo textextent
 Blockedby:  14705         |       Patch:  1                      
  Blocking:                |  
---------------------------+----------------------------------------------- -
Changes (by VZ):

  * status:  confirmed => closed
  * resolution:  => fixed

Comment:

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

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

 Closes #14706.