Re: [lmi] Wrong wxFlexGridSizer usage in about_dialog.cpp

[Vadim Zeitlin]

On Thu, 16 Apr 2015 04:08:22 +0000 Greg Chicares <address@hidden> wrote:

GC> In revision 4745 I introduced a magic constant with value 450:
GC>   
http://svn.savannah.nongnu.org/viewvc/lmi/trunk/about_dialog.cpp?root=lmi&r1=4744&r2=4745
GC> I'm not sure where that value came from, but it looks awful at 192 DPI.

 This is unsurprising, any hard coded value in pixels will look bad in at
least some resolutions (and the attempts to find a compromise can only make
it look bad in all resolutions).

GC> It looks a lot better if I double the value to 900, but that'll probably
GC> make it look awful at 96 DPI, and magic constants are a code smell anyway.
GC> What's the right way to accomplish the aesthetic goals spelled out in this
GC> old thread?

 The simplest fix using the latest wxWidgets would be to replace 450 with
"html_window->FromDIP(wxSize(450, 0)).x" where "DIP" stands for "device
independent pixel". This would basically just scale 450 by the DPI/96 and
so would preserve exactly the same behaviour at the default DPI and look
decent at 192. One problem with the expression above is that it's really
too clumsy and I have already added locally (but not pushed to the main
wxWidgets repository yet) an overload allowing to use just simpler and
shorter "html_window->FromDIP(450)".


 Another fix, which would IMHO be slightly preferable, would be to avoid
hard coding 450 at all and express the desired width in terms of the width
of characters used in the window. This would take both the font size and
DPI into account and also just seems to make more sense: it's more natural
to say that the window should be ~N characters wide rather than M pixels.

 I'm not sure how wide exactly should the window be, but in my testing
using "65*html_window->GetCharWidth()" seems to look well. Here is the full
trivial patch:

---------------------------------- >8 --------------------------------------
diff --git a/about_dialog.cpp b/about_dialog.cpp
index c0c7493..9fd4a2f 100644
--- a/about_dialog.cpp
+++ b/about_dialog.cpp
@@ -70,7 +70,7 @@ int AboutDialog::ShowModal()
         );
     html_window->SetBorders(0);
     html_window->SetPage(license_notices_as_html());
-    html_window->GetInternalRepresentation()->Layout(450);
+    
html_window->GetInternalRepresentation()->Layout(65*html_window->GetCharWidth());
     int const width  = html_window->GetInternalRepresentation()->GetWidth ();
     int const height = html_window->GetInternalRepresentation()->GetHeight();
     html_window->SetMinSize(wxSize(width, height));
---------------------------------- >8 --------------------------------------


 BTW, the same problem -- with hard coding pixel values -- also applies to
e.g. "3" used for the borders in the same AboutDialog. It's less severe, of
course, as nobody cares about the exact border values anyhow, but would be
even simpler to fix, especially with my (still...) upcoming patches for
DPI-independent borders in wxSizer in wxWidgets, please let me know if
you'd like me to make a patch fixing this code.

 Thanks,
VZ