Opened 3 years ago

Closed 20 months ago

#13902 closed enhancement (fixed)

Fixed wxGTK/Windows build with Visual C++ and MinGW

Reported by: kosenko Owned by:
Priority: normal Milestone: 3.0.0
Component: wxGTK Version:
Keywords: Cc: vaclavslavik, kosenko
Blocked By: Blocking:
Patch: yes

Description

I have fixed wxGTK build under Windows using Visual C++ and MinGW compilers with Win32 (not X11) backend.
I have tested in Visual C++ 2008 and MinGW/cmd.exe GCC 4.5.2 under Windows XP SP3.

wxGTK port under Windows can be useful:

A) to get same widgets behavior under Linux and Windows (GNOME look&feel under Windows)
B) for applications under Windows that have strong GTK+ third party dependencies
C) for debugging under Windows wxGTK-based applications

Key changes:

  1. __WXMSW__ macro was replaced with __WINDOWS__ one where it was required in non-GUI code. It is because of wxGTK is not based on wxMSW widgets.
  2. Bakefiles was modified with same approach as wxUniv port. It looks ugly but it works.
  3. Two *.c files was renamed to *.cpp ones because of Visual C++ has problems with mixing C and C++ in PCH.
  4. I have used Windows Resources (RC) instead of XPM because first it is included in executables and it is native under Windows. Also I have added wxHAS_RESOURCE_FILES macro to indicate if current platform supports resource files.
  5. Disabled some features:
    1. WebView/GTK+ (wxWebView) and GStreamer(wxMediaCtrl) looks like not implemented under Windows.
    2. wxGLCanvas, wxUIActionSimulator and partially keyboard handling has strong dependencies on X11 code.
  6. Enabled some Windows features: DIB, Dialup and OLE Automation.

You can build wxGTK/Windows port in the same way as wxUniv one:

  • Set WXGTK=1 in command line tools (mingw and nmake)
  • Select "GTK+ Debug" in Visual C++

I have used following GTK+ binaries:
http://ftp.gnome.org/pub/gnome/binaries/win32/gtk+/2.24/gtk+-bundle_2.24.8-20111122_win32.zip
Before compilation you need to add include folders:
<you_gtk_path>\include\gtk-2.0
<you_gtk_path>\include\glib-2.0
<you_gtk_path>\include\cairo
<you_gtk_path>\include\pango-1.0
<you_gtk_path>\include\gdk-pixbuf-2.0
<you_gtk_path>\include\atk-1.0
<you_gtk_path>\lib\glib-2.0\include
<you_gtk_path>\lib\gtk-2.0\include
and libs folder: <you_gtk_path>\lib
wxGTK under Windows uses gtk-win32-2.0.lib gdk-win32-2.0.lib pangocairo-1.0.lib gdk_pixbuf-2.0.lib cairo.lib pango-1.0.lib gobject-2.0.lib gthread-2.0.lib glib-2.0.lib, this libs is linked automatically using bakefiles.

Latest source codes is available in
https://github.com/kosenko/wxWidgets/tree/gtkwin

Attachments (8)

gtkwin.diff.7z download (50.0 KB) - added by kosenko 3 years ago.
Zipped patch without regenerated makefiles to fit wxTrac attachment file size limitation
samples_minimal.png download (17.8 KB) - added by kosenko 3 years ago.
Minimal sample
samples_calendar.png download (13.0 KB) - added by kosenko 3 years ago.
Calendar sample
samples_dialogs_about.png download (40.3 KB) - added by kosenko 3 years ago.
About dialog sample
samples_xrc.png download (63.6 KB) - added by kosenko 3 years ago.
XRC sample
gtkwin2.diff download (33.8 KB) - added by kosenko 2 years ago.
Latest diff
gtkwin3.diff download (24.4 KB) - added by kosenko 21 months ago.
Latest diff
gtkwin4.diff download (22.0 KB) - added by kosenko 21 months ago.
Latest diff

Download all attachments as: .zip

Change History (59)

Changed 3 years ago by kosenko

Zipped patch without regenerated makefiles to fit wxTrac attachment file size limitation

Changed 3 years ago by kosenko

Minimal sample

Changed 3 years ago by kosenko

Calendar sample

Changed 3 years ago by kosenko

About dialog sample

Changed 3 years ago by kosenko

XRC sample

comment:1 follow-up: Changed 3 years ago by vadz

  • Milestone set to 3.0
  • Status changed from new to confirmed

Thanks, this can definitely be useful.

I'm going to look at the patch in more details later but for now I have just one question: can we avoid generating "GTK+" configurations in MSVC projects by default? I.e. I think ideal would be to have some possibility to do it (using some define on bakefile_gen command line or something in Bakefiles.local.bkgen perhaps?) but not have it in the default/official projects as IMHO this is going to confuse people.

Also, I wonder why do we have TOOLKIT==MSW when GTK is used under MSW. I think it would be more logical to set TOOLKIT==GTK and test for PLATFORM_MSW.

In any case, the descriptions of __WINDOWS__ and __WXMSW__ in docs/doxygen/mainpages/const_cpp.h need to be updated as they don't mean the same thing any more. And wxHAS_RESOURCE_FILES should be documented there as well (thanks for adding it BTW, we should have had it since a long time ago).

comment:2 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by kosenko

Replying to vadz:

I.e. I think ideal would be to have some possibility to do it (using some define on bakefile_gen command line or something in Bakefiles.local.bkgen perhaps?) but not have it in the default/official projects as IMHO this is going to confuse people.

Maybe it would be better to place this files into build/gtk folder.

can we avoid generating "GTK+" configurations in MSVC projects by default?

I am not familiar with neither bakefiles nor Python but at the first glance it looks like *.bkgen generated file paths can't depend on makefiles options:
https://bakefile.svn.sourceforge.net/svnroot/bakefile/bakefile/trunk/src/bakefile_gen.py

Also, I wonder why do we have TOOLKIT==MSW when GTK is used under MSW. I think it would be more logical to set TOOLKIT==GTK and test for PLATFORM_MSW.

Yes, unfortunately when I try to use GTK toolkit under Windows:

  • build/bakefiles/config.bkl

    diff --git a/build/bakefiles/config.bkl b/build/bakefiles/config.bkl
    index 84812d3..3ee91af 100644
    a b it if SHARED=1 unless you know what you are doing. 
    432432 
    433433        <set var="WXTOPDIR"/> <!-- to be overridden on bakefile cmd line --> 
    434434 
     435        <set var="WIN32_TOOLKIT"> 
     436            <if cond="WXGTK=='0'">MSW</if> 
     437            <if cond="WXGTK=='1'">GTK</if> 
     438        </set> 
     439        <set var="WIN32_TOOLKIT_LOWERCASE"> 
     440            <if cond="WXGTK=='0'">msw</if> 
     441            <if cond="WXGTK=='1'">gtk</if> 
     442        </set> 
     443 
    435444        <set var="TOOLKIT" overwrite="0"> 
    436445            <if cond="FORMAT=='msevc4prj'">WINCE</if> 
    437446            <if cond="FORMAT=='msvs2005prj' and MSVS_PLATFORMS=='pocketpc2003'">WINCE</if> 
    438447            <if cond="FORMAT=='msvs2008prj' and MSVS_PLATFORMS=='pocketpc2003'">WINCE</if> 
    439             <if cond="FORMAT=='msvs2005prj' and MSVS_PLATFORMS=='win32'">MSW</if> 
    440             <if cond="FORMAT=='msvs2008prj' and MSVS_PLATFORMS=='win32'">MSW</if> 
    441             <if cond="FORMAT not in ['msevc4prj','msvs2005prj','msvs2008prj'] and PLATFORM_WIN32=='1'">MSW</if> 
     448            <if cond="FORMAT=='msvs2005prj' and MSVS_PLATFORMS=='win32'">$(WIN32_TOOLKIT)</if> 
     449            <if cond="FORMAT=='msvs2008prj' and MSVS_PLATFORMS=='win32'">$(WIN32_TOOLKIT)</if> 
     450            <if cond="FORMAT not in ['msevc4prj','msvs2005prj','msvs2008prj'] and PLATFORM_WIN32=='1'">$(WIN32_TOOLKIT)</if> 
    442451            <if cond="PLATFORM_OS2=='1'">PM</if> 
    443452        </set> 
    444         <set var="TOOLKIT_LOWERCASE">$(TOOLKIT.lower())</set> 
     453        <set var="TOOLKIT_LOWERCASE"> 
     454            <if cond="FORMAT=='msevc4prj'">wince</if> 
     455            <if cond="FORMAT=='msvs2005prj' and MSVS_PLATFORMS=='pocketpc2003'">wince</if> 
     456            <if cond="FORMAT=='msvs2008prj' and MSVS_PLATFORMS=='pocketpc2003'">wince</if> 
     457            <if cond="FORMAT=='msvs2005prj' and MSVS_PLATFORMS=='win32'">$(WIN32_TOOLKIT_LOWERCASE)</if> 
     458            <if cond="FORMAT=='msvs2008prj' and MSVS_PLATFORMS=='win32'">$(WIN32_TOOLKIT_LOWERCASE)</if> 
     459            <if cond="FORMAT not in ['msevc4prj','msvs2005prj','msvs2008prj'] and PLATFORM_WIN32=='1'">$(WIN32_TOOLKIT_LOWERCASE)</if> 
     460            <if cond="PLATFORM_OS2=='1'">pm</if> 
     461        </set> 
    445462        <set var="TOOLKIT_VERSION"/> 
    446463        <set var="HOST_SUFFIX"/> 
    447464        <set var="EXTRACFLAGS"/> 

I get "name '__subdir' is not defined" bakefile error during msvc and mingw makefiles generation. msvs2003prj, msvs2005prj and msvs2008prj project files generated without errors.

In any case, the descriptions of __WINDOWS__ and __WXMSW__ in docs/doxygen/mainpages/const_cpp.h need to be updated as they don't mean the same thing any more. And wxHAS_RESOURCE_FILES should be documented there as well.

Done.

wxHAS_RESOURCE_FILES is 1 under Windows and OS/2. But there is Mac OS X resources and GLib resource framework that is not supported by this predefined symbol. So maybe wxHAS_RESOURCE_FILES should be renamed to wxHAS_WINDOWS_RESOURCE_FILES (and ignore OS/2) or wxHAS_NAMED_RESOURCES (in comparison with XPM raw strings)?

comment:3 Changed 3 years ago by vadz

We could indeed have MSVC projects for building wxGTK in build/gtk but I still wonder if this is really necessary. People not afraid of building wxGTK under MSW should be capable of running bakefile to generate them too. And it would always be possible to build it from the command line, wouldn't it?

I.e. I'd just exclude wxGTK configurations from the project files by default and tell people how to enable them somewhere.

Concerning __subdir error I unfortunately have no idea... Maybe Vaclav can see something wrong with the above?

Finally, for the name of the #define, what about wxHAS_ICONS_IN_RESOURCES? This is precise and still not too long.

comment:4 follow-up: Changed 3 years ago by kosenko

PLATFORM_MSW (not PLATFORM_WIN32)
building wxGTK under MSW

Sounds like 'building wxGTK under wxCocoa' to me. Maybe we should use 'Windows' instead of 'MSW' in this case?

And it would always be possible to build it from the command line, wouldn't it?
I.e. I'd just exclude wxGTK configurations from the project files by default and tell people how to enable them somewhere.

Do you mean to enable wxGTK in mingw and msvc command line makefiles in build/msw folder and disable it in msvs2003prj, msvs2005prj and msvs2008prj project files by default?

Concerning __subdir error I unfortunately have no idea... Maybe Vaclav can see something wrong with the above?

Sent invitation.

Finally, for the name of the #define, what about wxHAS_ICONS_IN_RESOURCES? This is precise and still not too long.

This predefined symbol should work with wxIcon, wxBitmap, wxCursor and any other resource type. So maybe wxHAS_WINDOWS_RESOURCES? It is not too long and we can add another symbol when other (non Windows) types will be supported by wxWidgets.

comment:5 in reply to: ↑ 4 Changed 3 years ago by vadz

Replying to kosenko:

PLATFORM_MSW (not PLATFORM_WIN32)
building wxGTK under MSW

Sounds like 'building wxGTK under wxCocoa' to me. Maybe we should use 'Windows' instead of 'MSW' in this case?

I'm not sure if renaming PLATFORM_MSW to PLATFORM_WINDOWS is not going to break something. The problem here is that there i really no good name for the standard Windows UI toolkit... "Windows" means both the OS and the GUI.

And it would always be possible to build it from the command line, wouldn't it?
I.e. I'd just exclude wxGTK configurations from the project files by default and tell people how to enable them somewhere.

Do you mean to enable wxGTK in mingw and msvc command line makefiles in build/msw folder and disable it in msvs2003prj, msvs2005prj and msvs2008prj project files by default?

Yes, exactly.

Finally, for the name of the #define, what about wxHAS_ICONS_IN_RESOURCES? This is precise and still not too long.

This predefined symbol should work with wxIcon, wxBitmap, wxCursor and any other resource type. So maybe wxHAS_WINDOWS_RESOURCES? It is not too long and we can add another symbol when other (non Windows) types will be supported by wxWidgets.

My problem is not that it's too long but that it's somewhat unclear. Maybe wxHAS_IMAGES_IN_RESOURCES? To me this means that the images are defined outside of the program code, whether we're under Windows or somewhere else.

comment:6 Changed 3 years ago by kosenko

I'm not sure if renaming PLATFORM_MSW to PLATFORM_WINDOWS is not going to break something.

Actually there is only PLATFORM_WIN32 in bakefiles and there is no plans to rename it.

The problem here is that there i really no good name for the standard Windows UI toolkit...

Currently for user32.dll and commctrl.dll based Windows GUI is used word MSW, but for GTK+ with WIN32 backend is used word GTK.

"Windows" means both the OS and the GUI.

Yes, but GUI can be either MSW or GTK, not both at the same time, i.e. wxWindowsGTK class is not derived from wxWindowsMSW one.

I mean:

  • Windows (__WINDOWS__) - Windows platform, inluding GUI (MSW or GTK/WIN32)
  • MSW (__WXMSW__) - native, user32.dll and commctr.dll based Windows GUI
  • GTK under Windows (__WXGTK__) - GTK+ with WIN32 backend.

In this case MSW should be used only for native Windows GUI, not as synonym of Windows platform. In many places in code MSW means platform and most pieces of my patch is renames from __WXMSW__ to __WINDOWS__, but I think we should at least try to not use word MSW as platform in conversation and documentation.

My problem is not that it's too long but that it's somewhat unclear. Maybe wxHAS_IMAGES_IN_RESOURCES? To me this means that the images are defined outside of the program code, whether we're under Windows or somewhere else.

I supposed to use this symbol for any resource type, even raw data type, not only images. Maybe we have more information after your more detailed look on the patch later. Currently we have as an options:

  • wxHAS_ICONS_IN_RESOURCES
  • wxHAS_IMAGES_IN_RESOURCES
  • wxHAS_NAMED_RESOURCES
  • wxHAS_RESOURCE_FILES
  • wxHAS_WINDOWS_RESOURCES
  • wxHAS_WINDOWS_RESOURCE_FILES

comment:7 follow-up: Changed 3 years ago by kosenko

Do you mean to enable wxGTK in mingw and msvc command line makefiles in build/msw folder and disable it in msvs2003prj, msvs2005prj and msvs2008prj project files by default?

Yes, exactly.


Vaclav said:

Is it is possible to generate wxGTK/Windows into mingw and msvc by default and has possibility to generate msvs2003prj, msvs2005prj and msvs2008prj using Bakefiles.local.bkgen?

No, but you can perfectly well do it in the bakefiles themselves, by doing some things conditionally on the format. See config.bkl for plenty of examples.


It looks like we can have wxGTK/Windows for some bakefile formats or do not have. But we can't enable it manually later using Bakefiles.local.bkgen.

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

I understand that you want to use __WINDOWS__ for the platform and __WXMSW__ for the GUI layer and while I don't really like it because it's rather artificial (MSW was always just a shorter synonym for Windows and nobody is going to understand the difference between them without reading the documentation and how many people do this...), I'm ok with this because I see no better solution.

What I wanted to say in comment:1 was just that it would be better to avoid using MSW for the TOOLKIT option value in wxGTK builds as this one is really for the GUI library used and so should clearly be GTK.


I supposed to use this symbol for any resource type, even raw data type, not only images.

But why? Currently we need this in order to determine whether XPMs should be embedded in the code or not. This is really only important for the images IMHO. To turn your argument around, if we need this later for something else we can always add a new symbol.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 3 years ago by vadz

  • Cc vaclavslavik added

Replying to kosenko:

Do you mean to enable wxGTK in mingw and msvc command line makefiles in build/msw folder and disable it in msvs2003prj, msvs2005prj and msvs2008prj project files by default?

Yes, exactly.


Vaclav said:

Is it is possible to generate wxGTK/Windows into mingw and msvc by default and has possibility to generate msvs2003prj, msvs2005prj and msvs2008prj using Bakefiles.local.bkgen?

No, but you can perfectly well do it in the bakefiles themselves, by doing some things conditionally on the format. See config.bkl for plenty of examples.


It looks like we can have wxGTK/Windows for some bakefile formats or do not have. But we can't enable it manually later using Bakefiles.local.bkgen.

I thought we could have -DGTK=0 by default for MSVC projects (so that the option wouldn't be used and no configurations would be generated for it) in Bakefiles.bkgen and that it would be enough to remove it to generate the projects with wxGTK configurations in them. Vaclav, shouldn't this work?

Of course, now I realize that it still would need to be done by editing Bakefiles.bkgen and not adding something to Bakefiles.local.bkgen which is not ideal... But I don't see any other way. And I'd really like to confuse people by adding wxGTK configurations to all the official projects.

comment:10 in reply to: ↑ 8 Changed 3 years ago by kosenko

Replying to vadz:

Currently we need this in order to determine whether XPMs should be embedded in the code or not. This is really only important for the images IMHO. To turn your argument around, if we need this later for something else we can always add a new symbol.

It is because I prefer to use only one predefined symbol if possible.

What about wxHAS_NAMED_RESOURCES? It works with any resource type from one side and means that resource is named (not embedded, not XPM) from another side.

comment:11 Changed 3 years ago by kosenko

I have disabled wxGTK/Windows build in Visual C++ project files.

Now we nice to have to replace "TOOLKIT=='MSW' and WXGTK=='1'" with "TOOLKIT=='GTK'" in bakefiles.

comment:12 follow-up: Changed 3 years ago by vadz

I'm not sure what to do about this patch... I'd like to apply it soon as it modifies many files and so risks becoming out of date quickly but I'd really like to avoid confusing (and not "to confuse" as I wrote above) people by having the extra configurations in the MSVC project files. Can we just change the makefile-based formats for now perhaps?

comment:13 in reply to: ↑ 12 Changed 3 years ago by kosenko

I already disable wxGTK/Windows build in Visual C++ project files (msvc6prj, msvs2003prj, msvs2005prj, msvs2008prj). And it exists only in msvc, mingw etc.

  • build/bakefiles/config.bkl

    diff --git a/build/bakefiles/config.bkl b/build/bakefiles/config.bkl
    index 84812d3..57ac4f4 100644
    a b  
    5454        </description> 
    5555    </option> 
    5656 
     57    <if cond="FORMAT in ['msvc6prj', 'msvs2003prj', 'msvs2005prj', 'msvs2008prj']"> 
     58        <set var="WXGTK">0</set> 
     59    </if> 
     60 

Also this patch can be used as a few small patches:

  1. Change bakefiles (and makefiles, and project files) and wxGTK/Windows-specific changes
  2. Replace __WXMSW__ with __WINDOWS__ (mostly in wxBase)
  3. Disable X11 code in wxGTK
  4. Use wxHAS_NAMED_RESOURCE

(2) and (3) can be reviewed and fixed now without applying any changes related with wxGTK/Windows build.
(4) I'll rename wxHAS_RESOURCE_FILES to wxHAS_NAMED_RESOURCE and fix documentation soon.
(1) is based on (2) and (3), but (2) and (3) is not based on (1) and (4).

So you can start review and try to apply (2) and (3).

comment:14 Changed 3 years ago by vadz

As always, I'd definitely prefer applying several small patches and (3) and (4) are completely uncontroversial, so I'd love to do this but -- and sorry if I'm missing something obvious -- where are these patches?

comment:15 Changed 3 years ago by kosenko

where are these patches?

There is no these patch files and it does never exists. All changes are placed on github:
https://github.com/kosenko/wxWidgets/tree/gtkwin
And I am trying to keep all changes up to date.

All these "patches" can be only logically extracted during review.

I'd like to apply it soon as it modifies many files and so risks becoming out of date quickly.

BTW I already have merge conflict because of r70489

comment:16 Changed 3 years ago by kosenko

I have renamed wxHAS_RESOURCE_FILES to wxHAS_NAMED_RESOURCES, update and test gtkwin branch on github

comment:17 Changed 3 years ago by vadz

The trouble is that I don't use the git repository at github for historical reasons (I have my own git-svn clone that predates it...) so it's going to be difficult for me to pick out these changes from it. I'll try to do it but if you can make patches from your branch, it would be simpler to apply for me for now (I do plan to switch to using github repo in the future).

TIA!

comment:18 Changed 3 years ago by kosenko

The trouble is that I don't use the git repository at github for historical reasons

You can use github repository only to work on this ticket without using it by default.

it's going to be difficult for me to pick out these changes from it

  1. I am testing gitwin branch as a whole. If I start to devide this branch to patches I can add more bugs then it has now.
  2. I am not sure how exactly this branch can be splitted to patches.
  3. This patches will be outdated very soon.

In most cases changes is trivial and can be easy reviewed. It would be nice if you apply changes that at least is undoubtedly correct.

PS. Steve Lamerton also started work on github:
https://github.com/wxWidgets/wxWidgets/network

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

Replying to vadz:

I thought we could have -DGTK=0 by default for MSVC projects (so that the option wouldn't be used and no configurations would be generated for it) in Bakefiles.bkgen and that it would be enough to remove it to generate the projects with wxGTK configurations in them. Vaclav, shouldn't this work?

Yes, it should.

comment:20 in reply to: ↑ 2 Changed 3 years ago by vaclavslavik

Replying to kosenko:

I get "name 'subdir' is not defined" bakefile error during msvc and mingw makefiles generation. msvs2003prj, msvs2005prj and msvs2008prj project files generated without errors.

Sounds like TOOLKIT is assumed to be a constant somewhere, for some part of the Windows stuff (which was historically true). Probably related to SETUP_H_SUBDIR.

comment:21 Changed 3 years ago by kosenko

Thank you for information.

I have replaced TOOLKIT=='MSW' and WXGTK=='1' with TOOLKIT=='GTK' in bakefiles and now you should build with TOOLKIT=GTK TOOLKIT_VERSION=2 command line arguments. WXGTK bakefile option was removed.

comment:22 Changed 3 years ago by kosenko

  • Blocked By 14035 added

comment:23 Changed 3 years ago by kosenko

  • Blocked By 14036 added

comment:24 Changed 3 years ago by vadz

  • Blocked By

(In #14036) Using wxCRT_StrnicmpA() directly like this is really ugly, could you please add wxStrnicmp() (in the same/similar way to the existing wxStricmp() in wx/wxcrt.h) and use it instead?

Thanks!

comment:25 Changed 3 years ago by vadz

  • Blocked By

(In #14035) wxGTK_CONV() works in the code of wxWindow-derived classes only as it uses m_font to convert using the correct encoding in non-Unicode build. So we should either just use utf8_str() directly here or perhaps ensure that wxMBFILES is always defined as 1 when using wxGTK. OTOH it's not really clear what should its value be as we have both native MSW file APIs (working with wchar_t* strings) and Glib/GTK file APIs (working with char* UTF-8 strings) so actually it's just not enough to fit both... So using utf8_str() directly is probably the best thing to do.

Could you please check my reasoning to see if I'm not missing something and update the patch if it's correct?

comment:26 Changed 3 years ago by kosenko

  • Blocked By

(In #14036) Done. wxStrnicmp() was already existed in wx-API.

comment:27 Changed 3 years ago by kosenko

  • Blocked By

(In #14035) Done. I have used utf8_str() because of changing wxMBFILES may be incompatible with other source codes. I think wxMSW and wxGTK/Windows should have similar non GUI code.

comment:28 Changed 3 years ago by vadz

  • Blocked By

(In #14035) I think utf8_str() should be used everywhere, even in the code where wxGTK_CONV() does compile because it doesn't make sense to use e.g. KOI-8 for the file names just because the label in a file dialog uses a font in KOI-8 encoding, does it?

But looking at the code further I see that it would be really better to use wxConvFileName as it's supposed to be customizable to be able to create file names in e.g. Latin-1 if really necessary. So perhaps the best would be to use mb_str(wxConvFile)? Of course, this is exactly what fn_str() does when wxMBFILES==1 so it's a bit stupid but if we don't do this, what's the point of having wxConvFile at all in wxGTK?

Another minor question: why are the inclusions of "wx/gtk/private.h" needed? Is it another Unix/Windows difference?

comment:29 Changed 3 years ago by VZ

  • Blocked By

(In #14036) (In [70674]) Use wxStrnicmp() instead of strncasecmp() in wxGTK code.

This fixes compilation under Windows where strncasecmp() is not available --
but portable wxStrnicmp() always is.

Closes #14036.

comment:30 Changed 3 years ago by kosenko

  • Blocked By

(In #14035) We are dealing here with GTK+ API only, not file system API. So I think file conversion rules is not applied here.

utf8_str() and wxGTK_CONV() has difference only in GTK+ ANSI build. What encoding is used by GTK+ API in ANSI build in affected functions? UTF8? So what conversion function I should use? utf8_str() everywhere?

Another minor question: why are the inclusions of "wx/gtk/private.h" needed? Is it another Unix/Windows difference?

I just forgot to remove these inclusions after removing wxGTK_CONV().

comment:31 Changed 3 years ago by vadz

  • Blocked By

(In #14035) GTK+ has only a single build and it does always use UTF-8 normally but it has an exception for the file names, e.g. look at http://developer.gnome.org/gdk-pixbuf/2.22/gdk-pixbuf-animation.html#gdk-pixbuf-animation-new-from-file where it clearly says "Name of file to load, in the GLib file name encoding". I am not really sure what does it mean but I think it can be something other than UTF-8 and this is why we have configurable wxConvFile.

comment:32 Changed 2 years ago by kosenko

Show test images on the single page
Minimal sampleCalendar sampleAbout dialog sampleXRC sample

comment:33 Changed 2 years ago by kosenko

  • Blocked By

(In #14035) > what's the point of having wxConvFile at all in wxGTK

It looks like there is not sense in using wxConvFile at all in wxGTK. wxConvFile is used in non-GUI code in wxBase to work with C runtime and GUI toolkit dependency can be dangerous. So we probably should have another string conversion class to work with GLib file name encoding (not CRT filename encoding) but this is out of scope in this ticket.

I have used UTF-8 under Windows and leave wxString::fn_str() under Unix-like systems. It is because of in all these function UTF-8 is used under Windows (G_OS_WIN32):
"an absolute filename specified in the GLib file name encoding, which is the on-disk file name bytes on Unix, and UTF-8 on Windows"
http://developer.gnome.org/glib/2.30/glib-URI-Functions.html#g-filename-to-uri

g_filename_to_uri:
http://git.gnome.org/browse/glib/tree/glib/gconvert.h?h=glib-2-30

gdk_pixbuf_animation_new_from_file:
http://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/gdk-pixbuf-animation.h?h=gdk-pixbuf-2-24

gdk_pixbuf_save, gdk_pixbuf_new_from_file:
http://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/gdk-pixbuf-core.h?h=gdk-pixbuf-2-24

gtk_file_chooser_set_current_folder, gtk_file_chooser_set_current_name, gtk_file_chooser_set_filename:
http://git.gnome.org/browse/gtk+/tree/gtk/gtkfilechooser.h?h=gtk-2-24

Also I have not reassign wxConvFile. So wxMBConv_win32 for wxConvFile will be actually used in wxGTK, wxMSW and wxBase.

comment:34 Changed 2 years ago by vadz

  • Blocked By

(In #14035) Thanks for the explanations, I agree with the logic of the patch, your links show quite convincingly that UTF-8 should be used with filenames under Windows, thanks again.

But having to write #ifdef G_OS_WIN32 everywhere is really ugly. Could we please have some wxGTK_CONV_FN(string) macro in include/wx/gtk/private.h doing it in one place only?

TIA!

comment:35 Changed 2 years ago by kosenko

  • Blocked By 14035, 14036 removed

comment:36 Changed 2 years ago by kosenko

  • Blocked By 14035 added

(In #14035) Done. But I think that wxGTK_CONV_FILENAME sounds better then wxGTK_CONV_FN because of FN can be interpreted as function, not filename.

BTW wxGTK_CONV macro argument probably should have round brackets:

  • include/wx/gtk/private.h

     
    2828extern const gchar *wx_pango_version_check(int major, int minor, int micro); 
    2929 
    3030#if wxUSE_UNICODE 
    31     #define wxGTK_CONV(s) s.utf8_str() 
     31    #define wxGTK_CONV(s) (s).utf8_str() 
    3232    #define wxGTK_CONV_ENC(s, enc) wxGTK_CONV((s)) 
    3333    #define wxGTK_CONV_FONT(s, font) wxGTK_CONV((s)) 
    3434    #define wxGTK_CONV_SYS(s) wxGTK_CONV((s)) 

comment:37 Changed 2 years ago by kosenko

  • Blocked By 14035 removed

comment:38 Changed 2 years ago by vadz

  • Status changed from confirmed to infoneeded_new

Kolya, is there anything remaining to be applied here or was it superseded by your later patches?

Changed 2 years ago by kosenko

Latest diff

comment:39 Changed 2 years ago by kosenko

  • Status changed from infoneeded_new to new

There is many remaining code snippets that is neither submitted nor applied yet. Latest difference you can see in my repository as usual.

comment:40 follow-up: Changed 21 months ago by vadz

Is there any chance of applying the rest of your changes before 2.9.5? It'd be really nice to get some testing for them before 3.0 as applying them later could always break something else (e.g. the always fragile Cygwin build).

Unfortunately the last patch (gtkwin2.diff) doesn't apply at all any more, at least partially because some of its changes have been already applied. I'm also still worried about the big number of #if checks that it adds and am afraid that it could make code maintenance more difficult and would prefer, as usual, to move toolkit-specific code in separate files compiled only for this toolkit.

BTW, if it's easier for you to submit your changes as pull requests on Github, we could try to do it like this.

TIA!

comment:41 in reply to: ↑ 40 Changed 21 months ago by kosenko

Unfortunately the last patch (gtkwin2.diff) doesn't apply at all any more, at least partially because some of its changes have been already applied.


You can get latest patch with

git diff origin/master --name-only|grep -v "\.vcproj$"|grep -v "\.dsp$"\
|grep -v "\.sln$"|grep -v "/[Mm]akefile\."|grep -v "build/msw/config\."\
|grep -v "^Makefile.in"|grep -v "autoconf_inc\.m4"|grep -v "^configure$"\
|xargs git diff origin/master --


And uncomment following code to disable wxGTK in Visual C++ project files as discussed before.

<!--if cond="FORMAT in ['msvc6prj', 'msvs2003prj', 'msvs2005prj', 'msvs2008prj']">
    <set var="TOOLKIT">MSW</set>
</if-->


Is there any chance of applying the rest of your changes before 2.9.5? It'd be really nice to get some testing for them before 3.0 as applying them later could always break something else (e.g. the always fragile Cygwin build).


Unfortunately Visual C++ makefiles with enabled wxGTK port create build rules for all ports (wxOSX/*, wxCocoa, wxMotif, wxPM) and I don't known how it can be fixed.
Also there is weird bakefile generation bug that I have fixed with

<if cond="FORMAT in ['borland','mingw','msvc','watcom']">
    <define-rule name="__mm-to-$(OBJEXT[1:])" extends="__any,compilation_rule">
    </define-rule>
</if>

If this bug is not closed wxGTK/WIN32 port will be shipped in the fully disable state by default.

I'm also still worried about the big number of #if checks that it adds and am afraid that it could make code maintenance more difficult and would prefer, as usual, to move toolkit-specific code in separate files compiled only for this toolkit.


Most of #if toolkit checks was applied eventually, some checks remain in include/wx/msw/private.h.

Sorry for delay in response, I don't get personal e-mail notifications on changes in my tickets.

Changed 21 months ago by kosenko

Latest diff

comment:42 follow-up: Changed 21 months ago by vadz

  • Cc kosenko added

Unfortunately I don't know how to fix the bakefile problem neither. OTOH if it only adds unnecessary targets to makefiles, it's not a big problem, the important thing is to keep the project files without extra configurations and, of course, not make these new targets dependencies of "all".

Other than that the patch looks good to me except for, indeed, these checks in wx/msw/private.h, it would be nice to split it in 2 files as was already done for a couple of other ones. But it's probably better to apply this before 3.0 in the current state than not apply it at all...

P.S. Let me try to add you to the CC list to see if it helps with your email notification problem. If it doesn't, it must mean that the trac mail server has trouble sending email to you, for some reason...

comment:43 in reply to: ↑ 42 Changed 21 months ago by kosenko

the important thing is [...] not make these new targets dependencies of "all".

wxGTK files are not compiled by default during build because of following checks in makefiles:

!if "$(TOOLKIT)" == "GTK" && "$(TOOLKIT_VERSION)" == "2"


the important thing is to keep the project files without extra configurations

As I wrote in comment:40 wxGTK in project files can be easy disabled. And they are disabled in gtkwin4.diff.

these checks in wx/msw/private.h, it would be nice to split it in 2 files as was already done for a couple of other ones

I have removed all unnecessary checks wx/msw/private.h and there are no new checks, but only one just slightly modified and the file end:

  • include/wx/msw/private.h

    diff --git include/wx/msw/private.h include/wx/msw/private.h
    index 6c10cfa..52c81a6 100644
    enum wxWinVersion 
    932932 
    933933WXDLLIMPEXP_BASE wxWinVersion wxGetWinVersion(); 
    934934 
    935 #if wxUSE_GUI 
     935#if wxUSE_GUI && defined(__WXMSW__) 
    936936 
    937937// cursor stuff 
    938938extern HCURSOR wxGetCurrentBusyCursor();    // from msw/utils.cpp 
    inline void *wxSetWindowUserData(HWND hwnd, void *data) 
    10691069 
    10701070#endif // __WIN64__/__WIN32__ 
    10711071 
    1072 #endif // wxUSE_GUI 
     1072#endif // wxUSE_GUI && __WXMSW__ 
    10731073 
    10741074#endif // _WX_PRIVATE_H_ 


Other than that the patch looks good to me

So patch probably can be reviewed again.

Changed 21 months ago by kosenko

Latest diff

comment:44 follow-up: Changed 20 months ago by vadz

Thanks for the update! I didn't review the bakefile changes but as long as they don't affect the existing builds I'm fine with applying them nevertheless.

Just 2 minor questions:

  1. Why do we test for WIN32 and not _WIN32? According to the MSDN only the latter is guaranteed to be defined.
  2. I think it should be fine to just remove the call to Connect(wxEVT_IDLE) from the dll sample, am I missing any good reason not to do it?

TIA!

comment:45 in reply to: ↑ 44 Changed 20 months ago by kosenko

  1. Why do we test for WIN32 and not _WIN32? According to the MSDN only the latter is guaranteed to be defined.

I have missed this. So probably we should check for _WIN32 and define WIN32 if it is not defined.

  1. I think it should be fine to just remove the call to Connect(wxEVT_IDLE) from the dll sample, am I missing any good reason not to do it?

It is just because void wxApp::OnIdle(wxIdleEvent& event) is defined only under wxMSW and I need to compile somehow this code.

comment:46 follow-up: Changed 20 months ago by vadz

I'm going to apply this with just a small change to common.bkl: I'd like to avoid including gtk/setup.h in wxMSW projects, so I've put a toolkit test around msvc-headers-setup-h. Unfortunately we still need to include univ/setup.h in all cases as WXUNIV==1 is not a weak condition but at least this doesn't make things worse than before.

Thanks again for your work on this, the one thing I'd really like to see is a docs/gtk/README.Win32 file (or docs/msw/gtk.txt?) explaining how to build wxGTK under Windows, it would be great if you could write it. BTW, perhaps you'd also like to post about this to wxBlog?

comment:47 Changed 20 months ago by VZ

(In [73289]) Remove connection of MyDllApp::OnIdle() handler in the dll sample.

This method didn't really exist, the code only worked because it connected to
wxApp::OnIdle() which exists in wxMSW but not the other ports.

Simply remove the apparently unnecessary call.

See #13902.

comment:48 Changed 20 months ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [73290]) Allow building wxGTK under Windows with MSVC.

Replace a few more WXMSW tests with WINDOWS ones and modify bakefiles
to allow specifying the toolkit to be built in wxMSW makefiles.

Closes #13902.

comment:49 in reply to: ↑ 46 Changed 20 months ago by kosenko

Replying to vadz:

I'm going to apply this with just a small change to common.bkl: I'd like to avoid including gtk/setup.h in wxMSW projects, so I've put a toolkit test around msvc-headers-setup-h. Unfortunately we still need to include univ/setup.h in all cases as WXUNIV==1 is not a weak condition but at least this doesn't make things worse than before.


When I'm trying to create Visual C++ project files with patch

  • build/bakefiles/config.bkl

    diff --git build/bakefiles/config.bkl build/bakefiles/config.bkl
    index 3644983..3d5dba2 100644
     
    6666            </option> 
    6767        </if> 
    6868 
    69         <if cond="FORMAT in ['msvc6prj', 'msvs2003prj', 'msvs2005prj', 'msvs2008prj']"> 
     69        <!--if cond="FORMAT in ['msvc6prj', 'msvs2003prj', 'msvs2005prj', 'msvs2008prj']"> 
    7070            <set var="TOOLKIT">MSW</set> 
    71         </if> 
     71        </if--> 
    7272    </if> 
    7373 
    7474    <option name="WXUNIV"> 


I get error during bakefile generation: build/bakefiles/common.bkl:442: error: 'TOOLKIT=='MSW'': only weak condition allowed in this context. It can be fixed with

  • build/bakefiles/common.bkl

    diff --git build/bakefiles/common.bkl build/bakefiles/common.bkl
    index c8b45a7..3355be7 100644
    $(TAB)copy "$(DOLLAR)(InputPath)" $(SETUPHDIR)\wx\setup.h 
    436436                </set> 
    437437            </if> 
    438438            <if cond="FORMAT!='msevc4prj'"> 
    439                 <!-- Unfortunately we have to include wx/univ/setup.h in both 
    440                      cases because WXUNIV==1 is not a weak condition, but at 
    441                      least don't include wxGTK setup.h when building wxMSW. --> 
    442                 <if cond="TOOLKIT=='MSW'"> 
    443                     <msvc-headers-setup-h> 
    444                         msw/setup.h 
    445                         univ/setup.h 
    446                     </msvc-headers-setup-h> 
    447                 </if> 
    448                 <if cond="TOOLKIT=='GTK'"> 
    449                     <msvc-headers-setup-h> 
    450                         gtk/setup.h 
    451                         univ/setup.h 
    452                     </msvc-headers-setup-h> 
    453                 </if> 
     439                <msvc-headers-setup-h> 
     440                    msw/setup.h 
     441                    gtk/setup.h 
     442                    univ/setup.h 
     443                </msvc-headers-setup-h> 
    454444                <set var="_custom_build_include_wx_msw_setup_h"> 
    455445                    <if cond="WXUNIV=='0' and TOOLKIT=='MSW'"> 
    456446                        $(msvc_copy_setup_h_script % 'msw\setup.h') 


BTW do you have rebuild all bakefiles after this change (for example sample project files). It looks like I have slightly different output of same bakefiles.

Thanks again for your work on this, the one thing I'd really like to see is a docs/gtk/README.Win32 file (or docs/msw/gtk.txt?) explaining how to build wxGTK under Windows, it would be great if you could write it. BTW, perhaps you'd also like to post about this to wxBlog?

OK ;)

comment:50 Changed 20 months ago by vadz

  • Resolution fixed deleted
  • Status changed from closed to reopened

Vaclav, could you please confirm there is no better fox this? I'd really like to avoid having wxGTK setup.h in wxMSW project files, this would confuse matters with setup.h even further (and as we all know they're pretty confusing already).

comment:51 Changed 20 months ago by vadz

  • Resolution set to fixed
  • Status changed from reopened to closed

I shouldn't have reopened this, there is a separate #14965 for this issue.

Note: See TracTickets for help on using tickets.