Opened 10 years ago

Last modified 9 years ago

#12733 infoneeded_new defect

Incorrect dialog units conversion

Reported by: okurtsev Owned by:
Priority: high Milestone:
Component: GUI-all Version: stable-latest
Keywords: dialog layout, wxXmlResource, LoadDialog, XML regression Cc: vaclavslavik
Blocked By: Blocking:
Patch: no

Description

From 2.9.(?) dialog layout after loading XML file hasn't appeared correctly.

There is nothing suspicious in XML-file, for example for the "Purchase" button

MS Studio .rc file -

PUSHBUTTON "Purchase",IDC_BUTTON_PURCHASE,123,66,50,18

XML

<object class="wxButton" name= "IDC_BUTTON_PURCHASE">

<pos>123,66d</pos> <size>50,18d</size>

<label>Purchase</label>

</object>

But their appearances are different in MS Studio resource editor(or wxWidgets 2.8.11) and in wxWidgets 2.9.2 application (see attachment).

Attachments (1)

dialog_xml.png download (30.5 KB) - added by okurtsev 10 years ago.
Appearances in MS Studio and in wxWidgets 2.9.2 app

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by okurtsev

Appearances in MS Studio and in wxWidgets 2.9.2 app

comment:1 Changed 10 years ago by vadz

  • Keywords regression added
  • Milestone set to 3.0

This is really strange, dialog units computation did change in 2.9 but it was supposed to make it more (and even exactly) compatible with Windows dialog units, not less so, see the comments before wxWindowBase::GetDlgUnitBase() in src/common/wincmn.cpp.

Could you please check what is returned by this function in your case? And how does it compare to the results of GetCharWidth() and GetCharHeight() which were used as the base size in 2.8?

comment:2 Changed 10 years ago by okurtsev

I've just checked, 

wxWindowBase::GetDlgUnitBase() in 2.9.2 returns {6, 14}

GetCharWidth(), GetCharHeight() in 2.8.11 returns 6 and13 respectively and dialog looks exactly as I expect.

Thank you for answering, but why milestone is 3.0? It is actually a very bad surprise. Now loading from XML appears unusable, because there are not only wrong sizes of controls, but also controls appears shifted down, so I thing it is critical actually. It is possible to adjust, of course, with trial and error method, but for software with a lot of dialogs it is a disaster. Please prompt some workaround if possible. 

comment:3 Changed 10 years ago by vadz

  • Cc vaclavslavik added
  • Component changed from XRC to GUI-all
  • Milestone changed from 3.0 to 2.9.2
  • Status changed from new to confirmed
  • Summary changed from Wrong dialog layout wxXmlResource::LoadDialog from XML file to Incorrect dialog units conversion

Thanks for testing, it looks like the dialog units don't count descent (which is returned as part of the height by GetTextExtent() but ignored by GetCharHeight()) and so we should do the same in wxPrivate::GetAverageASCIILetterSize(). Vaclav, anything I'm missing here? FWIW the URL mentioned in the comments does use tmHeight too.

And I agree, this is a serious regression, let's fix this in 2.9.2.

comment:4 Changed 10 years ago by rensy1

This bug is killing me. :( Everything text comes up too large on my box (win7 x64). I've had to revert the whole project back to 2.8.x to get it out the door.

comment:5 follow-up: Changed 10 years ago by vadz

  • Status changed from confirmed to infoneeded_new

In fact I was wrong in my previous comment, MSDN code use TEXTMETRICS::tmHeight which does include the descent. So it looks like our current code should be correct -- but if it's different from the old version, it's still a problem, of course :-(

Could you please check if this patch fixes the problem:

  • include/wx/private/window.h

    diff --git a/include/wx/private/window.h b/include/wx/private/window.h
    index 3084665..d80d3fe 100644
    a b inline wxSize GetAverageASCIILetterSize(const T& of_what) 
    2929    const wxStringCharType *TEXT_TO_MEASURE =
    3030        wxS("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz");
    3131
    32     wxSize s = of_what.GetTextExtent(TEXT_TO_MEASURE);
     32    wxSize s;
     33    int descent;
     34    of_what.GetTextExtent(TEXT_TO_MEASURE, &s.x, &s.y, &descent);
    3335    s.x = (s.x / 26 + 1) / 2;
     36
     37    // For compatibility with Windows (and previous wxWidgets versions)
     38    // behaviour, don't count descent as part of the text height for the dialog
     39    // units computations.
     40    s.y -= descent;
     41
    3442    return s;
    3543}
    3644

?

I'm not at all sure if it's correct though... Without this patch I get 7*15 pixels for 4*8 in dialog units on my (standard Win7 x64) system. With it I get 7*12. And ::GetDialogBaseUnits() returns 8*16 here.

And according to MSDN (see the section "Converting from DLU to relative pixels and back") the 7*15 value (current one) is actually correct (the row in the left hand table has 28*30 for 16 DLUs).

What does this (press F1 in the minimal sample) do on your system(s):

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    index fb8f721..cd88e25 100644
    a b void MyFrame::OnQuit(wxCommandEvent& WXUNUSED(event)) 
    185185
    186186void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event))
    187187{
     188#if 0
    188189    wxMessageBox(wxString::Format
    189190                 (
    190191                    "Welcome to %s!\n"
    void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event)) 
    197198                 "About wxWidgets minimal sample",
    198199                 wxOK | wxICON_INFORMATION,
    199200                 this);
     201#else
     202    wxDialog dlg(this, -1, "Title");
     203    RECT r = { 0, 0, 16, 16 };
     204    if ( !::MapDialogRect(dlg.GetHWND(), &r) )
     205    {
     206        wxLogLastError("MapDialogRect");
     207    }
     208
     209    const wxPoint p = dlg.ConvertDialogToPixels(wxPoint(16, 16));
     210    wxLogMessage("16*16 wx dialog units = %d*%d pixels\n"
     211                 "16*16 MSW dialog units = %d*%d pixels",
     212                 p.x, p.y,
     213                 r.right, r.bottom);
     214#endif
    200215}

?

comment:6 in reply to: ↑ 5 Changed 10 years ago by vaclavslavik

Replying to vadz:

but if it's different from the old version, it's still a problem, of course :-(

Sorry, what do you mean? Of course it's different from the old 2.8 version of this code, that's kind of the point.

comment:7 Changed 10 years ago by vadz

Sorry, I got confused (once again). The problem is that it's different from MSVS resource editor while it was apparently the same in 2.8 seemingly indicating that the 2.8 version was correct -- and the new one is not.

Also, I thought we only changed the horizontal DLU component computation, not the vertical one in 2.9.

comment:8 follow-up: Changed 9 years ago by vadz

  • Milestone 2.9.2 deleted

After looking at this again it became apparent (thanks to Vaclav) that the two screenshots use different fonts. Hence, unsurprisingly, the dialog units are different too as they're font-dependent.

So the question becomes: why did the font change?

And, more generally, what's the simplest way of reproducing the problem? Do you have the simplest possible test XRC showing it?

comment:9 in reply to: ↑ 8 Changed 9 years ago by vaclavslavik

Replying to vadz:

So the question becomes: why did the font change?

As I noted, wx was fixed to use correct default font in r61790 (see also #11008), so that explains the difference between 2.8 and 2.9 if the font wasn't set explicitly.

Note: See TracTickets for help on using tickets.