wxTextMeasure implementation

[Manolo]

Hi
Thank you Vadim for your review of my proposed patch.
I'm sure you understand that I have not dedicated such many years to
work with wx internals as you did. So, I'm prone to make faults.
Anyhow, I want to comment some subjects.


For what I understood from other post, you prefer to have this new
class internal. At least in a beginning.
Documenting it for public API is not hard (just copying explanations
from existing wxDC docs). Or perhaps it is indeed a hard job, because
current doc's are not, IMHO, fully true. For example, GetTextExtent()
in wxGTK is really a member of wxWindowDC and not of wxDC. Separating
all text measures docs from wxDC and wxWindow and placing them in the
wxTextMeasure class doc may be clearer for the reader. He/She will know
that internally wx uses only one way of measuring; no doubt if wxDC and
wxWindow work differently.

My intention was just to gather the code, replacing existing one with a
call to the new class member. Then I realized that this class could be
used anywhere, and that wxWindow gets improved with new features, not
just GetTextExtent().
If this reason makes you think of a public API, OK. But if so, please
tell what kind of doc you like: adding or reworking. "Reworking" means
for me replacing these text measure explanations with a laconic "See
wxTextMeasure".


The 'm_selfCalled' var allows a simple call like
myWindow->GetTextExtent(...)
to work (init, do the job, release resources). And because other
calls like GetMultiLineTextExtent() or GetLargestStringExtent() make
intensive use of GetTextExtent(), avoid "init & release" repetition.

Your approach of public GetTextExtent() calls Init(), then calls
virtual DoGetTextExtent() and at last calls Restore() IMO makes the
code more difficult to understand for a future reader. It becomes a
labyrinth of base and derived classes, public and virtual members.
For those who don't know the "run path", I expose an example:
wxDC myDC;
myDC->GetTextExtent(text...);
Internals: wxDC is generic. It stores a pointer 'm_pimpl' to the
abstract class wxDCImpl.
In wxGTK, it depends on using Gtk2 or Gtk3. In Gtk2 the inheritance is
wxWindowDCImpl child of wxGTKDCImpl child of wxDCImpl. In Gtk3 is
wxWindowDCImpl child of wxGTKCairoDCImpl child of wxGCDCImpl child of
wxDCImpl.
myDC->GetTextExtent() calls m_pimpl->DoGetTextExtent() which is the
virtual member of the proper version of wxWindowDCImpl. This does the job.
And now, let's add wxTextMeasure.
Because of backward compatibility, we keep wxDC::GetTextExtent(). So
wxWindowDCImpl creates a wxTextMeasure object and calls the public
wxTextMeasure::GetTextExtent() which should call the platform dependant
virtual DoGetTextExtent().

All of this labyrinth can be avoided just by:
wxDC myDC;
wxTextMeasure myTm;
myTm->GetTextExtent(text,..., myDC...);
and providing virtual platform-dependant versions of GetTextExtent().

I suppose anyone who wants to write a version of GetTextExtent() must
know the internals. Either must be aware of the m_selfCalled flag or
must be aware of overriding DoGetTextExtent() instead of GetTextExtent(),
and that both functions will be used in the "run path".

I agree using your approach allows overloading public methods. Do we
really need them?


Using
wxTextMeasure tm(this, m_font);
return tm.GetPartialTextExtents(text, widths, m_scaleX);
is clearer, yes. But
wxTextMeasure tm(this, m_font);
tm.GetPartialTextExtents(text, widths1, m_scaleX);
SetFont(newfont);
tm.GetPartialTextExtents(text, widths2, m_scaleX);
is wrong, because the 'tm' is not aware of new font.
This case is possible if the user does not know internals.
tm.GetPartialTextExtents(text, widths, m_scaleX, font, this);
has no doubt and uses always the right font.


On the rest of your review, you are right. I was some asleep when
I sent the patch and didn't reviewed it properly.

I put the header in wx.h because many basic controls (button, etc)
need some text measurement.

g++ does not warn about virtual dtor in wxTextMeasureBase because
I didn't use a pointer to this class.

I passed a pointer instead of a const reference because some time
ago I read http://www.gotw.ca/gotw/081.htm Since then, if I want
to be sure that no copy is done, despite of if the compiler is smart
or not, I use a pointer and take care of not modifying the object
myself, instead of using 'const'.
I may agree with using 'const' for a member that is going to be
overridden who knows how many times. Is this the case?


Regards,
Manolo

[Vadim Zeitlin]

On Mon, 01 Oct 2012 22:00:50 +0200 Manolo wrote:

M> I'm sure you understand that I have not dedicated such many years to
M> work with wx internals as you did. So, I'm prone to make faults.

Hi,

Everybody makes mistakes (myself definitely included), it's just that it's
much simpler to notice them when reviewing the code written by somebody
else than when looking at your own code. I hope you don't take the number
of comments I made about your patch badly.

M> For what I understood from other post, you prefer to have this new
M> class internal. At least in a beginning.

OK, I'm actually not sure about this, but let's do it like this. If it is
internal, then the headers should go into include/wx/private and
include/wx/$PORT/private directories, you should remove the DLL export
macros and can include windows.h directly from wx/msw/private/textmeasure.h.

M> The 'm_selfCalled' var allows a simple call like
M> myWindow->GetTextExtent(...)
M> to work (init, do the job, release resources). And because other
M> calls like GetMultiLineTextExtent() or GetLargestStringExtent() make
M> intensive use of GetTextExtent(), avoid "init & release" repetition.

Yes, I understand this, but I don't like this approach because it relies
on m_selfCalled being set/updated correctly by all the derived classes and
it's too error-prone. Worse, if you forget to do it, the code still works,
just inefficiently. This is not good.

M> Your approach of public GetTextExtent() calls Init(), then calls
M> virtual DoGetTextExtent() and at last calls Restore() IMO makes the
M> code more difficult to understand for a future reader. It becomes a
M> labyrinth of base and derived classes, public and virtual members.

There is probably some merit in this argument and it could indeed be less
clear for a very novice programmer but this is a perfectly widely used and
well-understood C++ technique which is also already used in many places in
wxWidgets itself (albeit not as many as I'd like). So I really don't think
we should discard it just because of this. A novice will hopefully find it
not very difficult to learn. And everybody else won't have any troubles at
all with (and how many novices exactly go spelunking in wxWidgets sources
anyhow?).

M> For those who don't know the "run path", I expose an example:
M> wxDC myDC;
M> myDC->GetTextExtent(text...);
M> Internals: wxDC is generic. It stores a pointer 'm_pimpl' to the
M> abstract class wxDCImpl.
M> In wxGTK, it depends on using Gtk2 or Gtk3. In Gtk2 the inheritance is
M> wxWindowDCImpl child of wxGTKDCImpl child of wxDCImpl. In Gtk3 is
M> wxWindowDCImpl child of wxGTKCairoDCImpl child of wxGCDCImpl child of
M> wxDCImpl.
M> myDC->GetTextExtent() calls m_pimpl->DoGetTextExtent() which is the
M> virtual member of the proper version of wxWindowDCImpl. This does the job.
M> And now, let's add wxTextMeasure.
M> Because of backward compatibility, we keep wxDC::GetTextExtent(). So
M> wxWindowDCImpl creates a wxTextMeasure object and calls the public
M> wxTextMeasure::GetTextExtent() which should call the platform dependant
M> virtual DoGetTextExtent().

This is indeed rather complicated but:

1. You absolutely don't need to know about this when you just use wxWidgets.
2. Most of the extra steps have nothing to do with wxTextMeasure, e.g. the
fact that wxDC uses wxDCImpl is really completely orthogonal to it.

M> All of this labyrinth can be avoided just by:
M> wxDC myDC;
M> wxTextMeasure myTm;
M> myTm->GetTextExtent(text,..., myDC...);
M> and providing virtual platform-dependant versions of GetTextExtent().

I don't like this at all. This makes the public API more confusing and for
what benefit exactly?

M> I suppose anyone who wants to write a version of GetTextExtent() must
M> know the internals. Either must be aware of the m_selfCalled flag or
M> must be aware of overriding DoGetTextExtent() instead of GetTextExtent(),
M> and that both functions will be used in the "run path".

In the latter case you can't override GetTextExtent() because it's not
virtual. Also, again, it's a very common C++ idiom to have public Foo() and
private virtual DoFoo(). If somebody doesn't know about it yet, it would be
a good occasion to learn about it.

M> I agree using your approach allows overloading public methods. Do we
M> really need them?

Yes, I think it's much more convenient.

M> Using
M> wxTextMeasure tm(this, m_font);
M> return tm.GetPartialTextExtents(text, widths, m_scaleX);
M> is clearer, yes.

And this is important.

M> But
M> wxTextMeasure tm(this, m_font);
M> tm.GetPartialTextExtents(text, widths1, m_scaleX);
M> SetFont(newfont);
M> tm.GetPartialTextExtents(text, widths2, m_scaleX);
M> is wrong, because the 'tm' is not aware of new font.

Well, you really need to make an effort to do this, don't you? When you
construct wxTextMeasure you say "I want to measure the text on this window
(or DC) using this font". Obviously things won't work correctly if you
change the font later.

I think it's a non-problem, we don't devise API impossible to misuse
(especially private API) but just API that is difficult to misuse
accidentally. And there is nothing accidental in the above code.

If you really want to allow it, you could have wxTextMeasure::SetFont()
but, frankly, I wouldn't bother.

M> This case is possible if the user does not know internals.
M> tm.GetPartialTextExtents(text, widths, m_scaleX, font, this);
M> has no doubt and uses always the right font.

Yes, and the parameter list is perfectly unreadable (you chose a short
form, in your patch there are calls with 7 parameters, 2 of which can be
indistinguishable from each other NULLs).

M> I put the header in wx.h because many basic controls (button, etc)
M> need some text measurement.

If it's a private header, it's not even a question, it can't appear in
public wx.h.

M> I passed a pointer instead of a const reference because some time
M> ago I read http://www.gotw.ca/gotw/081.htm Since then, if I want
M> to be sure that no copy is done, despite of if the compiler is smart
M> or not, I use a pointer and take care of not modifying the object
M> myself, instead of using 'const'.
M> I may agree with using 'const' for a member that is going to be
M> overridden who knows how many times. Is this the case?

wxWidgets convention is to use const reference for all parameters and to
only use const pointers for the parameters that can be NULL. I didn't read
the GotW above but I can't believe Herb Sutter would recommend using (raw)
pointers and I don't see any mention of it from a quick glance at the above
URL. Anyhow, there is no copying being done by the compiler there and, as I
mentioned, I think this class should be made non-copyable anyhow to make it
completely impossible.

Regards,
VZ

[Manolo]

>
> Everybody makes mistakes (myself definitely included), it's just that it's
> much simpler to notice them when reviewing the code written by somebody
> else than when looking at your own code. I hope you don't take the number
> of comments I made about your patch badly.

Don't worry. I'm sure you have missed some other bugs on my patch ;)

>
> M> For what I understood from other post, you prefer to have this new
> M> class internal. At least in a beginning.
>
> OK, I'm actually not sure about this, but let's do it like this. If it is
> internal, then the headers should go into include/wx/private and
> include/wx/$PORT/private directories, you should remove the DLL export
> macros and can include windows.h directly from wx/msw/private/textmeasure.h.

OK

> clear for a very novice programmer but this is a perfectly widely used and
> well-understood C++ technique which is also already used in many places in
> wxWidgets itself (albeit not as many as I'd like).

Yes, it is a widely extended technique. It became a paradigm. I'm
against the use of a paradigm without weighing pros and cons for each
case. Following Occam's razor principle, I prefer using the minimum set
of members and derived classes.

> M> All of this labyrinth can be avoided just by:
> M> wxDC myDC;
> M> wxTextMeasure myTm;
> M> myTm->GetTextExtent(text,..., myDC...);
> M> and providing virtual platform-dependant versions of GetTextExtent().
>
> I don't like this at all. This makes the public API more confusing and for
> what benefit exactly?

I don't understand this. The public API GetTextExtent() stays as is.
The overridden versions is where the text is measured. The benefit is
the save of the use of another member DoGetTextExtent(), for the case
GetTextExtend() won't be overloaded but needs to be overridden because
it is platform dependant.

Regards,
Manolo

[Vadim Zeitlin]

On Tue, 02 Oct 2012 18:07:34 +0200 Manolo wrote:

M> > clear for a very novice programmer but this is a perfectly widely used and
M> > well-understood C++ technique which is also already used in many places in
M> > wxWidgets itself (albeit not as many as I'd like).
M>
M> Yes, it is a widely extended technique. It became a paradigm. I'm
M> against the use of a paradigm without weighing pros and cons for each
M> case. Following Occam's razor principle, I prefer using the minimum set
M> of members and derived classes.

I don't think you should try to save on the number of classes and methods.
To take a simpler example, it's like saying, for "classic" procedural
programming style, that you should have as few functions as possible. This
would be clearly wrong as it would basically mean that you'd write all your
code as one huge function which is exactly the opposite of the good
approach. The same thing, more or less, applies to OO programming: you
shouldn't try to have as few classes or as few virtual methods as possible,
you should try to have as well-defined/encapsulated/isolated/lightly
coupled classes as possible.

Again, the major advantage of this technique are that there is a clear
separation between public interface of the class and the interface used by
the derived classes. This has several beneficial effects, that I already
mentioned but which I could list again if you wish, and really has no
drawbacks. OTOH the alternative you used, with using a flag that needs to
be explicitly managed is much more brittle and less clear.

M> > M> All of this labyrinth can be avoided just by:
M> > M> wxDC myDC;
M> > M> wxTextMeasure myTm;
M> > M> myTm->GetTextExtent(text,..., myDC...);
M> > M> and providing virtual platform-dependant versions of GetTextExtent().
M> >
M> > I don't like this at all. This makes the public API more confusing and for
M> > what benefit exactly?
M>
M> I don't understand this. The public API GetTextExtent() stays as is.
M> The overridden versions is where the text is measured.

You'd add a new public GetTextExtent() overload. Even if it doesn't change
the existing overload, it would still make the public API less clear
because now you'd need to think about which overload do you need to use.

Also notice that for this to be possible at all, wxTextMeasure would need
to be public which we decided against for now.

M> The benefit is the save of the use of another member DoGetTextExtent(),
M> for the case GetTextExtend() won't be overloaded but needs to be
M> overridden because it is platform dependant.

There is no benefit in "saving on DoGetTextExtent()". It really doesn't
cost anything to have a virtual method. Reducing the number of virtual
methods is simply a wrong goal and it's not worth pursuing it.

Regards,
VZ

[Manolo]

> you should try to have as well-defined/encapsulated/isolated/lightly
> coupled classes as possible.

This is what I called the "minimum set".
I'm talking about using a feature just because it's a paradigm.
For example, a Point class
class Point
{
public:
int x, y;
};
It is very clear what 'x' and 'y' mean. Nobody is going to get into
troubles because of using myPoint.x = 3;
But the paradigm says "make x,y private and add setters and accessors".
So we add some Get/Set member functions. This is rarely useful, but adds
more code and [sometimes] more confusion.
Of course, in other cases, the paradigm is really true.

FWIW, you have decided that wxWidgets follows some paradigms. OK. We can
live with them.
I'll rework the patch the way you like.

Regards,
Manolo

[Vadim Zeitlin]

On Tue, 02 Oct 2012 19:18:43 +0200 Manolo wrote:

M> > you should try to have as well-defined/encapsulated/isolated/lightly
M> > coupled classes as possible.
M>
M> This is what I called the "minimum set".
M> I'm talking about using a feature just because it's a paradigm.
M> For example, a Point class
M> class Point
M> {
M> public:
M> int x, y;
M> };
M> It is very clear what 'x' and 'y' mean. Nobody is going to get into
M> troubles because of using myPoint.x = 3;
M> But the paradigm says "make x,y private and add setters and accessors".
M> So we add some Get/Set member functions. This is rarely useful, but adds
M> more code and [sometimes] more confusion.
M> Of course, in other cases, the paradigm is really true.

Sorry but this is a bad example. It's a bad idea to add useless accessors
that don't abstract anything but not because it creates any confusion -- it
really doesn't, I've never seen anybody confused by what SetX() or GetY()
does. It's bad simply because it's unnecessary while what I propose to do
here is definitely not unnecessary... I do agree with you that adding
complexity for complexity sake is a bad idea, but this is not at all what
I'd like to do here.


M> FWIW, you have decided that wxWidgets follows some paradigms. OK. We can
M> live with them. I'll rework the patch the way you like.

It's unfortunate that my arguments didn't convince you :-( I'd really
prefer if we could agree to do something that we both believe to be correct
instead of you doing what you think is wrong just because I said so.

Again, my main points are:

1. Setting the flag is error-prone and actually more confusing because,
unlike the order of function calls, it's not immediately obvious from
reading the code when is it supposed to be set/reset/checked. At least
for me having a (public) function that prepares the class to be used
and a private one that operates on the object in the known prepared
state is much more clear.
2. Having public Foo() and private DoFoo() allows to:
(a) Cleanly decouple public API from what needs to be implemented by the
derived classes.
(b) As a nice side effect allows to have overloaded public methods.

If you are still not convinced, you should really explain what do you
disagree with and why, otherwise I don't know what else can I say
unfortunately.

Regards,
VZ

[Manolo]

No. I don't think your approach is wrong and mine is right. No.
I think both approaches are right.

My mates say that I have the ability to find mistakes. They also
say that my ideas can be so different from typical ones that they
seem wrong at a first glance. And more than once these ideas from
mine are really wrong ;)

Your approach rings my "attention: paradigm" bell. I agree it
follows C++ spirit more than mine. But the goal is to make code
easy and functional. If a future "patcher" is going to forget to
update the flag, or refuse to do a patch because the time required
to read more code... well, I don't know. I'd bet to the latter.
Can you convince me that the former is a more frequent case?

If I would be sure that you are wrong I'd explain why it does not work.
But this is not the case. I just tried to explain that something
(complexity) smells not so good and my instinct alerts me.

I'll do the patch the way you say, because "we both think it is
correct" (although we differ in guessing where a future "patcher"
will fail), while we don't agree in my approach.

Regards,
Manolo

[Vadim Zeitlin]

On Wed, 03 Oct 2012 01:51:01 +0200 Manolo wrote:

M> Your approach rings my "attention: paradigm" bell. I agree it
M> follows C++ spirit more than mine. But the goal is to make code
M> easy and functional. If a future "patcher" is going to forget to
M> update the flag, or refuse to do a patch because the time required
M> to read more code... well, I don't know. I'd bet to the latter.
M> Can you convince me that the former is a more frequent case?

I can't convince you. What I can say is that

(a) I'm not sure if people who don't know C++ at a good enough level to
recognize (possibly without knowing how it's called) this idiom should
be contributing to wxWidgets as they're bound to be very inexperienced.
If you work in C++ for any length of time, you just must have
encountered it already.

(b) In the approach with the flag the likeliest outcome is that it will be
just ignored (i.e. not set). And as the code would still compile and
work -- just inefficiently -- this could well pass unnoticed. I prefer
something that works automatically.

M> But this is not the case. I just tried to explain that something
M> (complexity) smells not so good and my instinct alerts me.

Again, I agree that complexity is bad. But having layers, e.g. separate
public and private functions in this case, is exactly how we manage
complexity in programming. We can have more things but as each of them is
simpler, the total complexity is less. This is "divide and conquer" at its
best.

M> I'll do the patch the way you say, because "we both think it is
M> correct" (although we differ in guessing where a future "patcher"
M> will fail), while we don't agree in my approach.

OK, I guess I'll have to settle on that.

Good luck and thanks for working on this,
VZ

[Manolo]

Here I am again
I have reworked the patch.
I can build wxGTK and compile samples like combo or richtext.

Now I'm trying wxUniv on gtk. My first time in this port.
wx libs compile well. No sample can compile.
I get "undefined reference" to anything related to wxTextMeasure.

/home/manolo/wxSVN/b_univ/lib/libwx_gtk2univu_core-2.9.a(corelib_gtk_window.o):
In function `wxWindowGTK::DoGetTextExtent(wxString const&, int*, int*,
int*, int*, wxFont const*) const':
/home/manolo/wxSVN/b_univ/../src/gtk/window.cpp:3104: undefined
reference to `wxTextMeasure::wxTextMeasure(wxWindow const*, wxFont const*)'
/home/manolo/wxSVN/b_univ/../src/gtk/window.cpp:3105: undefined
reference to `wxTextMeasureBase::GetTextExtent(wxString const&, int*,
int*, int*, int*)'
/home/manolo/wxSVN/b_univ/lib/libwx_gtk2univu_core-2.9.a(corelib_gtk_window.o):
In function `~wxTextMeasure':
/home/manolo/wxSVN/b_univ/../include/wx/gtk/private/textmeasure.h:24:
undefined reference to `vtable for wxTextMeasure'


Because I know nothing about wxUniv internals, I don't know if
I must add more entries to files.bkl, or if I must... what?

Can anyone help?

TIA
Manolo

[Vadim Zeitlin]

On Thu, 04 Oct 2012 01:00:27 +0200 Manolo wrote:

M> Now I'm trying wxUniv on gtk. My first time in this port.
M> wx libs compile well. No sample can compile.
M> I get "undefined reference" to anything related to wxTextMeasure.

Which variable(s) did you add the new source files implementing it to
in build/bakefiles/files.bkl? I think you must have added it to
GTK[2]_SRC, but it should go to GTK_LOWLEVEL_SRC to be used for wxUniv
as well.

Good luck,
VZ

[Manolo]

El 04/10/12 12:58, Vadim Zeitlin escribi�:

I've added these 3 entries:
at GUI_CMN_SRC
src/common/textmeasurecmn.cpp
at GTK_SRC
src/gtk/textmeasure.cpp
at MSW_SRC
src/msw/textmeasure.cpp

Do you say also adding to both GTK2_SRC and GTK_LOWLEVEL_SRC?
And to MSW_LOWLEVEL_SRC?

Thanks,
Manolo

[Vadim Zeitlin]

On Thu, 04 Oct 2012 14:04:45 +0200 Manolo wrote:

M> I've added these 3 entries:
M> at GUI_CMN_SRC
M> src/common/textmeasurecmn.cpp

This one is fine.

M> at GTK_SRC
M> src/gtk/textmeasure.cpp
M> at MSW_SRC
M> src/msw/textmeasure.cpp
M>
M> Do you say also adding to both GTK2_SRC and GTK_LOWLEVEL_SRC?
M> And to MSW_LOWLEVEL_SRC?

No, they should be added to GTK_LOWLEVEL_SRC and MSW_LOWLEVEL_SRC
*instead* of adding them to GTK_SRC and MSW_SRC. The full wxFoo port uses
both FOO_LOWLEVEL_SRC and FOO_SRC while wxUniv/Foo uses FOO_LOWLEVEL_SRC
but UNIV_SRC instead of FOO_SRC.

Regards,
VZ

[Manolo]

El 04/10/12 14:09, Vadim Zeitlin escribi�:

OK.
I hope it wont break compilation in other ports. I can not test them.

Thanks,
Manolo

[Vadim Zeitlin]

On Thu, 04 Oct 2012 14:17:36 +0200 Manolo wrote:

M> I hope it wont break compilation in other ports. I can not test them.

I'll test compilation under Mac before committing but you could test e.g.
wxUniv/X11. It's sufficiently different from wxGTK to show the problems
like the one from the original patch version.

TIA,
VZ

[Manolo]

El 04/10/12 14:19, Vadim Zeitlin escribi�:

You knew it, didn't you? ;)
wxUniv/X11 compiles. Combo sample compiles and "almost" works.
I say "almost" because texts are well measured. But...

* Pango-WARNING**: All fonts fallbacks failed!!!!
On the window, instead of each character I see a rectangle.

* When I manage to open and enlarge "combo comparison" and click
in a drop, it crashes. The BT is:
#0 0xb7fdd424 __kernel_vsyscall () (??:??)
#1 0xb758ddde raise(sig=5) (../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42)
#2 0xb770c447 wxTrap() (../src/common/appbase.cpp:1024)
#3 0xb7ae4f0c wxGUIAppTraitsBase::ShowAssertDialog(this=0x80b0ed0,
msg=...) (../src/common/appcmn.cpp:459)
#4 0xb770d087 ShowAssertDialog(file=..., line=219, func=..., cond=...,
msgUser=..., traits=0x80b0ed0) (../src/common/appbase.cpp:1265)
#5 0xb770ba41 wxAppConsoleBase::OnAssertFailure(this=0x80b0100,
file=0x812308c L"../src/common/arrstr.cpp", line=219, func=0x8123c54
L"Index", cond=0x81216ec L"bCase && !bFromEnd", msg=0x8124254 L"search
parameters ignored for auto sorted array") (../src/common/appbase.cpp:761)
#6 0xb770c5d1 wxDefaultAssertHandler(file=..., line=219, func=...,
cond=..., msg=...) (../src/common/appbase.cpp:1065)
#7 0xb770ccac wxOnAssert(file=0xb7814b4f "../src/common/arrstr.cpp",
line=219, func=0xb7815079 "Index", cond=0xb7814b3c "bCase && !bFromEnd",
msg=0xb7814a7c L"search parameters ignored for auto sorted array")
(../src/common/appbase.cpp:1150)
#8 0xb77114ab wxArrayString::Index(this=0x8121e68, str=..., bCase=false,
bFromEnd=false) (../src/common/arrstr.cpp:219)
#9 0xb7a923e4 wxListBox::FindString(this=0x8121000, s=..., bCase=false)
(../src/univ/listbox.cpp:239)
#10 0xb7a8a46b wxComboListBox::SetStringValue(this=0x8121000, value=...)
(../src/univ/combobox.cpp:148)
#11 0xb7afca62 wxComboCtrlBase::OnSetValue(this=0x8120790, value=...)
(../src/common/combocmn.cpp:2716)
#12 0xb7afcb3f wxComboCtrlBase::SetValueByUser(this=0x8120790,
value=...) (../src/common/combocmn.cpp:2736)
#13 0xb7afc3b8 wxComboCtrlBase::HidePopup(this=0x8120790,
generateEvent=true) (../src/common/combocmn.cpp:2516)
#14 0xb7af81a8 wxComboPopup::Dismiss(this=0x8121220)
(../src/common/combocmn.cpp:673)
#15 0xb7a8a537 wxComboListBox::OnLeftUp(this=0x8121000, event=...)
(../src/univ/combobox.cpp:179)
#16 0xb770b441 wxAppConsoleBase::HandleEvent (this=0x80b0100,
handler=0x8121000, func=(void (wxEvtHandler::*)(wxEvtHandler * const,
wxEvent &) (../src/common/appbase.cpp:591)
#17 0xb770b49d wxAppConsoleBase::CallEventHandler(this=0x80b0100,
handler=0x8121000, functor=..., event=...) (../src/common/appbase.cpp:603)
#18 0xb77ff07f wxEvtHandler::ProcessEventIfMatchesId(entry=...,
handler=0x8121000, event=...) (../src/common/event.cpp:1373)
#19 0xb77fe12e wxEventHashTable::HandleEvent(this=0xb7e3af80, event=...,
self=0x8121000) (../src/common/event.cpp:979)
#20 0xb77ff500 wxEvtHandler::TryHereOnly(this=0x8121000, event=...)
(../src/common/event.cpp:1564)
#21 0xb7800aa3 wxEvtHandler::TryBeforeAndHere(this=0x8121000, event=...)
(../include/wx/event.h:3348)
#22 0xb77ff33e wxEvtHandler::ProcessEventLocally(this=0x8121000,
event=...) (../src/common/event.cpp:1497)
#23 0xb77ff2d6 wxEvtHandler::ProcessEvent(this=0x8121000, event=...)
(../src/common/event.cpp:1470)
#24 0xb7c1316a wxScrollHelperEvtHandler::ProcessEvent(this=0x8120280,
event=...) (../src/generic/scrlwing.cpp:212)
#25 0xb77ff3c3 wxEvtHandler::DoTryChain(this=0x8121b90, event=...)
(../src/common/event.cpp:1529)
#26 0xb77ff354 wxEvtHandler::ProcessEventLocally(this=0x8121b90,
event=...) (../src/common/event.cpp:1497)
#27 0xb77ff2d6 wxEvtHandler::ProcessEvent(this=0x8121b90, event=...)
(../src/common/event.cpp:1470)
#28 0xb77ff542 wxEvtHandler::SafelyProcessEvent(this=0x8121b90,
event=...) (../src/common/event.cpp:1577)
#29 0xb7bcd52a wxWindowBase::HandleWindowEvent(this=0x8121000,
event=...) (../src/common/wincmn.cpp:1498)


Well. I told you I'm an expert on discovering bugs. I will not fix these
new issues. By now, it goes beyond my capacity. If anyone is interested,
let him dig a bit and open tickets.

Regards,
Manolo

[Manolo]

> M> at GUI_CMN_SRC
> M> src/common/textmeasurecmn.cpp
>
> This one is fine.
>
> M> at GTK_SRC
> M> src/gtk/textmeasure.cpp
> M> at MSW_SRC
> M> src/msw/textmeasure.cpp
> M>
> M> Do you say also adding to both GTK2_SRC and GTK_LOWLEVEL_SRC?
> M> And to MSW_LOWLEVEL_SRC?
>
> No, they should be added to GTK_LOWLEVEL_SRC and MSW_LOWLEVEL_SRC
> *instead* of adding them to GTK_SRC and MSW_SRC. The full wxFoo port uses
> both FOO_LOWLEVEL_SRC and FOO_SRC while wxUniv/Foo uses FOO_LOWLEVEL_SRC
> but UNIV_SRC instead of FOO_SRC.
>

There must be something I'm missing here, because wxMSW builds, but trying to
compile 'combo' sample I get again "undefined reference" errors.
Are you sure an entry at MSW_SRC is not needed?
What about headers?

By the way, is there a Mingw makefile for wxUniv/MSW?

TIA
Manolo

[Vadim Zeitlin]

On Thu, 04 Oct 2012 22:19:24 +0200 Manolo wrote:

M> > No, they should be added to GTK_LOWLEVEL_SRC and MSW_LOWLEVEL_SRC
M> > instead of adding them to GTK_SRC and MSW_SRC. The full wxFoo port uses
M> > both FOO_LOWLEVEL_SRC and FOO_SRC while wxUniv/Foo uses FOO_LOWLEVEL_SRC
M> > but UNIV_SRC instead of FOO_SRC.
M> >
M> There must be something I'm missing here, because wxMSW builds, but trying to
M> compile 'combo' sample I get again "undefined reference" errors.

Are you sure you rebaked all the project files? It's easy to get
confused...

M> Are you sure an entry at MSW_SRC is not needed?

Yes, the full set of core library sources includes both MSW_SRC and
MSW_LOWLEVEL_SRC so you just need to add the file to one of those.

M> What about headers?

As long as they're private, they don't need to be added to files.bkl at
all.

M> By the way, is there a Mingw makefile for wxUniv/MSW?

Yes, it's the same one as for wxMSW. Just specify WXUNIV=1 (or
--enable-universal if using configure).

Regards,
VZ

[Vadim Zeitlin]

On Thu, 04 Oct 2012 18:15:16 +0200 Manolo wrote:

M> * Pango-WARNING**: All fonts fallbacks failed!!!!
M> On the window, instead of each character I see a rectangle.

This looks like a bug but almost certainly one unrelated to your changes.
I wonder when did this break down :-(

M> * When I manage to open and enlarge "combo comparison" and click
M> in a drop, it crashes. The BT is:
...
M> #9 0xb7a923e4 wxListBox::FindString(this=0x8121000, s=..., bCase=false)
M> (../src/univ/listbox.cpp:239)

OK, we shouldn't be using bCase here... I'll check in a blind fix for
this.

Regards,
VZ

[Manolo]

> M> > No, they should be added to GTK_LOWLEVEL_SRC and MSW_LOWLEVEL_SRC
> M> > instead of adding them to GTK_SRC and MSW_SRC. The full wxFoo port uses
> M> > both FOO_LOWLEVEL_SRC and FOO_SRC while wxUniv/Foo uses FOO_LOWLEVEL_SRC
> M> > but UNIV_SRC instead of FOO_SRC.
> M> >
> M> There must be something I'm missing here, because wxMSW builds, but trying to
> M> compile 'combo' sample I get again "undefined reference" errors.
>
> Are you sure you rebaked all the project files? It's easy to get
> confused...

Yes, I do. And yes, it's easy to get confused.
I found what was wrong: I forgot to implement constructors!!. Hard to
believe, but so is it.

> M> By the way, is there a Mingw makefile for wxUniv/MSW?
>
> Yes, it's the same one as for wxMSW. Just specify WXUNIV=1 (or
> --enable-universal if using configure).

Thanks.
Manolo

[Manolo]

Hi
I'm finishing my tests, so I'll send a new version for this patch soon.

By the way, combo sample with wxUniv/msw fails (open comparison dialog,
drop one of
"native" combos at right) because the assert in
src/common/combocmn.cpp::521 OnDismiss().

It's a pity wxUniv is still so buggy. I believe that the future of apps is
not based in their
efficiency, but in it's look. Many, many people like to use "something new
and different".
This fashion sensation is achieved with the look. While some of it may be
done with themes,
more adaptable/fancy/funny/customizable/animate/etc. controls might be done
with wxUniv.

Regards,
Manolo

[Manolo]

Another wxUniv/msw bug, this time with richtextsample:
Just after the frame is built, a size event is handled and, trying on
DoMoveWindow(7,68,0,21):

#0 ?? wxWindowMSW::DoSetSize (this=0x5a5cb20, x=7, y=68, width=0,
height=21, sizeFlags=36) (../../src/msw/window.cpp:2046)
#1 00780FC5 wxWindowBase::SetSize(this=0x5a5cb20, x=5, y=66, width=0,
height=21, sizeFlags=36) (../../include/wx/window.h:260)
#2 00539B38 wxSizerItem::SetDimension(this=0x5a4eec0, pos_=...,
size_=...) (../../src/common/sizer.cpp:502)
#3 0053EC4D wxBoxSizer::RecalcSizes(this=0x5a5afb8)
(../../src/common/sizer.cpp:2377)
#4 0053B0C6 wxSizer::Layout(this=0x5a5afb8) (../../src/common/sizer.cpp:979)
#5 007D2E42 wxSizer::SetDimension(this=0x5a5afb8, pos=..., size=...)
(../../include/wx/sizer.h:679)
#6 007D2EC5 wxSizer::SetDimension(this=0x5a5afb8, x=0, y=0, width=0,
height=92) (../../include/wx/sizer.h:686)
#7 00563C96 wxWindowBase::Layout(this=0x5a5a6b8)
(../../src/common/wincmn.cpp:2398)
#8 00447137 wxRichTextStyleListCtrl::OnSize(this=0x5a5a6b8)
(../../src/richtext/richtextstyles.cpp:1102)
#9 00663887 wxAppConsoleBase::HandleEvent (this=0x5a139a8,
handler=0x5a5a6b8, func=(void (wxEvtHandler::*)(wxEvtHandler * const,
wxEvent &) (../../src/common/appbase.cpp:591)
#10 006638DF wxAppConsoleBase::CallEventHandler(this=0x5a139a8,
handler=0x5a5a6b8, functor=..., event=...) (../../src/common/appbase.cpp:603)
#11 006719DD wxEvtHandler::ProcessEventIfMatchesId(entry=...,
handler=0x5a5a6b8, event=...) (../../src/common/event.cpp:1373)
#12 00670BDB wxEventHashTable::HandleEvent(this=0xa6c8a8, event=...,
self=0x5a5a6b8) (../../src/common/event.cpp:979)
#13 00671DD1 wxEvtHandler::TryHereOnly(this=0x5a5a6b8, event=...)
(../../src/common/event.cpp:1564)
#14 0077D705 wxEvtHandler::TryBeforeAndHere(this=0x5a5a6b8, event=...)
(../../include/wx/event.h:3348)
#15 00671C42 wxEvtHandler::ProcessEventLocally(this=0x5a5a6b8,
event=...) (../../src/common/event.cpp:1497)
#16 00671BF2 wxEvtHandler::ProcessEvent(this=0x5a5a6b8, event=...)
(../../src/common/event.cpp:1470)
#17 00671DFE wxEvtHandler::SafelyProcessEvent(this=0x5a5a6b8, event=...)
(../../src/common/event.cpp:1577)
#18 005614C8 wxWindowBase::HandleWindowEvent(this=0x5a5a6b8, event=...)
(../../src/common/wincmn.cpp:1498)
#19 004F48E6 wxWindowMSW::HandleSize(this=0x5a5a6b8, wParam=0)
(../../src/msw/window.cpp:5159)
#20 004EFF47 wxWindowMSW::MSWHandleMessage(this=0x5a5a6b8,
result=0x23e9ec, message=5, wParam=0, lParam=6356988)
(../../src/msw/window.cpp:2753)
#21 004F153E wxWindowMSW::MSWWindowProc(this=0x5a5a6b8, message=5,
wParam=0, lParam=6356988) (../../src/msw/window.cpp:3609)
#22 00512D72 wxWindow::MSWWindowProc(this=0x5a5a6b8, message=5,
wParam=0, lParam=6356988) (../../src/univ/winuniv.cpp:1492)
#23 004EFB24 wxWndProc(hWnd=0x3004a0, message=5, wParam=0,
lParam=6356988) (../../src/msw/window.cpp:2703)
#24 7E398734 USER32!GetDC() (C:\WINDOWS\system32\user32.dll:??)
#25 003004A0 ?? () (??:??)
#26 00000005 ?? () (??:??)
#27 ?? ?? () (??:??)