Re: winemac: Add registry settings to make Option keys send Alt rather than accessing additional characters from the keyboard layout.

[Ken Thomases]

On Oct 9, 2013, at 4:49 PM, Phil Krylov wrote:

> Thanks!

You're welcome.

> Do you consider adding an option to stop interpreting Command as Alt?

I have not considered it.  What would be gained?  Do you want the Command key 
interpreted as the Windows key?  Do you want something else to happen when 
Command is pressed?

Cheers,
Ken

Re: winemac.drv: add fullscreen mode support for Mac OS X 10.7+

[Ken Thomases]

Hi,

I finally got around to working on support for Cocoa full-screen mode in the 
Mac driver, based on the work of Kevin Eaves.  I've attached a new patch.  This 
patch can only be applied on top of the other Mac driver patches I just 
submitted to wine-patches.

Some changes from Kevin's original, in no particular oder:

* I have not used the user32 hack to increase the max tracking size and let 
windows grow so their non-frame area covers the screen.  Instead, the call to 
SetWindowPos() that results from a WINDOW_FRAME_CHANGED event uses 
SWP_NOSENDCHANGING for Cocoa-full-screen windows.  That prevents any immediate 
modification of the new window rect to be within the max tracking size.  
However, some programs (e.g. Guild Wars) end up moving the window again shortly 
afterward and then it gets constrained.  This results in a window that doesn't 
quite fill the screen, showing a plain background around the edges.  This isn't 
ideal.  As previously discussed, this may require a new driver entry point to 
allow it to override the size limits.  (Although I got slapped down for trying 
to add a similar entry point for some other work.)

* Cocoa understandably refuses to minimize a window that's in full-screen mode. 
 So, if Win32-land tries to programmatically minimize a full-screen window, 
Cocoa just immediately tells it that it's been unminimized.  This shouldn't 
come up much.  (One can access a window's system menu using the keyboard to 
test.)

* We can't distinguish the program trying to make a window Win32 full-screen 
vs. it merely echoing back the frame set by Cocoa full-screen.  So, we never 
consider a window to be in Win32 full-screen mode if it's in Cocoa full-screen 
mode.  That means that the menu and Dock auto-unhide.  Many (most?) apps will 
modify the window style in addition to just setting the frame such that it 
becomes incompatible with Cocoa full-screen mode.  In that case, the window is 
first taken out of Cocoa full-screen and then put into Win32 full-screen mode.  
The menu and Dock are properly hidden, but the window went back to its original 
space, which may not be what the user wants.

* I have added the standard menu item for controlling Cocoa full-screen, Enter 
Full Screen, to the Window menu.  Cocoa automatically renames that to Exit Full 
Screen and back as the window enters and exits full-screen mode.  As with other 
items in the Mac menus, I don't allow keyboard shortcuts that don't include 
Option among the modifiers.  So, I've used Command-Option-Control-F instead of 
just Command-Control-F.

* The menu item and the window widget are properly disabled when the window is 
disabled.

* The maximum tracking size set by the app for the window is respected in 
full-screen mode.  If the app leaves the default max tracking size alone, then 
the full-screen size is unconstrained (and so fills the screen) even though the 
Win32 default is slightly too small to allow that.

* If the app programmatically changes the window rect while the window is in 
Cocoa full-screen mode, that change is honored.  This may end up looking a bit 
odd, but is necessary for correctness.  Furthermore, the changed rect is 
maintained as the window exits full-screen mode, which is not what Cocoa would 
normally do.  (Cocoa restores the window to the size and position it had before 
entering full-screen.)  This is necessary when, for example, a game switches 
from windowed (in Cocoa full-screen mode) to Win32 full-screen.  It will often 
change the window style in such a way that forces it out of Cocoa full-screen 
and changes its size to fill the screen.  We don't want Cocoa negating that 
size change as it transitions out of Cocoa full-screen mode.

Programmatic changes of the window rect that occur during and shortly after the 
transition into full-screen are not interpreted as setting the frame that 
should be restored when exiting full-screen mode.  Those are probably just 
responses from Wine and the app to the changes that Cocoa has initiated.

* I set NSWindowCollectionBehaviorFullScreenAuxiliary on any windows which 
don't get NSWindowCollectionBehaviorFullScreenPrimary.  I'm not certain that 
this is right.  That flag is not as clearly documented as I would like.  My 
intent is to allow other Wine windows to share the space with a 
full-screen-primary window.

* WS_EX_TOOLWINDOW windows (utility windows, in Cocoa parlance) are not 
eligible for Cocoa full-screen.  This means that they get 
NSWindowCollectionBehaviorFullScreenAuxiliary as per the previous point, so 
they can share a space with a full-screen primary window.

* I have left out any attempt to reconcile Cocoa full-screen with changes to 
the display mode which result in the displays being captured.  I don't know of 
an app which does that while leaving its window eligible for Cocoa full-screen, 
although I haven't tested many yet.


I invite everybody who's interested to test and/or review.  Cocoa full-screen 
was introduced in

Re: [PATCH 3/3] winemac: Tell Wine when Cocoa has brought a window to the front.

[Ken Thomases]

On Oct 4, 2013, at 12:17 AM, Ken Thomases wrote:

> ---
> dlls/winemac.drv/cocoa_app.m|  8 ++--
> dlls/winemac.drv/cocoa_window.h |  1 +
> dlls/winemac.drv/cocoa_window.m | 16 +---
> dlls/winemac.drv/event.c|  5 +
> dlls/winemac.drv/macdrv.h   |  1 +
> dlls/winemac.drv/macdrv_cocoa.h |  1 +
> dlls/winemac.drv/window.c   | 12 
> 7 files changed, 39 insertions(+), 5 deletions(-)
> 
> <0003-winemac-Tell-Wine-when-Cocoa-has-brought-a-window-to.patch>

Don't commit this one.  It has caused some strange behavior on Mavericks.

The other two in the series are OK and useful independently of this one.

-Ken

Looking for an icon for the Mac driver: generic program running under Wine

[Ken Thomases]

Hi,

I have a need for a new icon in Wine and I'm hoping somebody with some 
graphics-design skills might be able to create it.

The Mac driver attempts to extract an icon from the executable to use for the 
Dock icon of its process.  This is also the icon that appears in the 
Command-Tab application switcher.  This often works, but not always.  When it 
doesn't, the Dock just uses the generic "Unix executable" icon from Mac OS X.  
You can see that here: .  It's not very nice.

I would prefer to have a Wine-specific generic program icon.


Any graphic designer care to take on the task of creating such an icon?


I'm told Wine has been using the Tango icons as the template for its.  Tango 
provides application-x-executable.svg at 
 that might be 
suitable.  That could be badged with the Wine glass logo to produce an icon for 
a program running under Wine.

On the other hand, if this icon were to be used only by the Mac driver, it 
might be more appropriate to use a Mac-themed icon.

Some time back, Jeremiah posted an archive of icons.  
  That 
included one which had a miniature Mac window badged with the Wine glass logo.  
I'm not sure of the copyright/licensing situation with that icon.  It's 
bitmap-only, not vector, but that might be fine for a Mac-only icon.  It does 
have some issues with rough edges around the window, rather than a smooth 
alpha-blended edge.

Although Mac app icons can go up to 1024x1024 pixels (512x512 "points" at 2x 
resolution for Retina displays), this icon would only appear in the Dock and 
the application switcher.  So, it needn't be larger than 256x256 pixels 
(128x128@2x).

Thanks,
Ken

Re: winemac: Reapply display modes when switching back to app after "escaping" with Command-Tab.

[Ken Thomases]

On Oct 1, 2013, at 11:24 PM, Ken Thomases wrote:

> May fix bug <http://bugs.winehq.org/show_bug.cgi?id=34475>.

Should fix bug <http://bugs.winehq.org/show_bug.cgi?id=34209>, too.

-Ken

Re: winex11: Don't trace a garbage value or read past end of caller's array in X11DRV_wglChoosePixelFormatARB().

[Ken Thomases]

On Sep 20, 2013, at 9:44 AM, Roderick Colenbrander wrote:

> That change looks suspicious. If switching to i+1 fixes a problem, the real
> problem is that the if-statement in question gets entered in the first
> place. Some bounds checking on the outer for-loop (the one which loops
> through nCfgs and nMaxFormats), is probably wrong then. I had issues with
> this part in the past, so there maybe a small typo (it may just be off by
> 1).

This has already been committed, but I'll clarify.  The old code looked like:

piFormats[pfmt_it++] = i + 1;
TRACE("at %d/%d found FBCONFIG_ID 0x%x (%d)\n",
  it + 1, nCfgs, fmt_id, piFormats[pfmt_it]);

The problem is that pfmt_it was incremented in the first statement so it is no 
longer correct in the TRACE.  It now refers to the array element after the one 
that was assigned.  That may read past the end of piFormats.  In any case, it 
will read an element which has not yet been assigned.

I could have moved the increment to after both statements.  Or I could have 
used "pfmt_it - 1" in the TRACE.  It seemed simplest to just use the value that 
we know is the found pixel format.

-Ken

Re: [PATCH 1/5] user32: Add a SetMinMaxInfo driver entry point.

[Ken Thomases]

On Sep 16, 2013, at 1:21 PM, Ken Thomases wrote:

> On Sep 16, 2013, at 12:55 PM, Alexandre Julliard wrote:
> 
>> It's better to avoid adding entry points that don't correspond to
>> Windows APIs. Instead you should request the info when you need it.
> 
> Cocoa makes use of the size limits "spontaneously", not just at the time when 
> the user begins to resize a window.  On OS X 10.7 and later, the user can 
> resize windows by their edges and the cursor changes to show the available 
> operations.  So the limits need to be set in advance and updated whenever the 
> app would respond differently to WM_GETMINMAXINFO.  Since I have no way of 
> knowing in advance when it might do so, I figured I would either have to poll 
> (more or less) or piggyback on user32's queries.
> 
> Also, it's not clear to me that apps will tolerate WM_GETMINMAXINFO occurring 
> at times when Windows would not send it.

Well, the first "app" that doesn't tolerate it is the user32 test suite.  Other 
than when a user starts to resize a window, any other time I could think of to 
send WM_GETMINMAXINFO causes test failures.

For now, I've sent new versions of the patches which only get the info at the 
start of a resize.  Until then, Cocoa won't know the size limits and may 
display incorrect resize cursors at the window edges.

Any suggestions on other times/ways I could query the min/max info without 
generating messages that will break the tests?

-Ken

Re: [PATCH 1/5] user32: Add a SetMinMaxInfo driver entry point.

[Ken Thomases]

On Sep 16, 2013, at 12:55 PM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> ---
>> dlls/user32/driver.c   |   12 
>> dlls/user32/user_private.h |1 +
>> dlls/user32/winpos.c   |2 ++
>> 3 files changed, 15 insertions(+), 0 deletions(-)
> 
> It's better to avoid adding entry points that don't correspond to
> Windows APIs. Instead you should request the info when you need it.

Cocoa makes use of the size limits "spontaneously", not just at the time when 
the user begins to resize a window.  On OS X 10.7 and later, the user can 
resize windows by their edges and the cursor changes to show the available 
operations.  So the limits need to be set in advance and updated whenever the 
app would respond differently to WM_GETMINMAXINFO.  Since I have no way of 
knowing in advance when it might do so, I figured I would either have to poll 
(more or less) or piggyback on user32's queries.

Also, it's not clear to me that apps will tolerate WM_GETMINMAXINFO occurring 
at times when Windows would not send it.

I'll rework the patches and I guess we'll find out.

-Ken

Re: [PATCH 3/6] user32: Export WINPOS_GetMinMaxInfo() as __wine_get_min_max_info() so it can be called by drivers.

[Ken Thomases]

On Sep 12, 2013, at 10:48 AM, Dmitry Timoshkov wrote:

> Ken Thomases  wrote:
> 
>>>> +@ cdecl __wine_get_min_max_info(long ptr ptr ptr ptr) WINPOS_GetMinMaxInfo
>>> 
>>> What's wrong with calling SendMessage(WM_GETMINMAXINFO) from the driver?
>> 
>> Because I would have to duplicate all of the rest of the logic in 
>> WINPOS_GetMinMaxInfo(), too.
> 
> Do you really need anything besides ptMaxSize? In any way even duplicating
> some of that code is cleaner than adding one more private export IMHO.

I don't use ptMaxSize currently.  I use ptMinTrackSize and ptMaxTrackSize.

As to which is cleaner, I guess I'll wait to see which Alexandre prefers.

-Ken

Re: [PATCH 3/6] user32: Export WINPOS_GetMinMaxInfo() as __wine_get_min_max_info() so it can be called by drivers.

[Ken Thomases]

On Sep 12, 2013, at 12:23 AM, Dmitry Timoshkov wrote:

> Ken Thomases  wrote:
> 
>> +@ cdecl __wine_get_min_max_info(long ptr ptr ptr ptr) WINPOS_GetMinMaxInfo
> 
> What's wrong with calling SendMessage(WM_GETMINMAXINFO) from the driver?

Because I would have to duplicate all of the rest of the logic in 
WINPOS_GetMinMaxInfo(), too.

-Ken

Re: winemac: Don't return characters for Ctrl-(non-letter symbol) keypresses from ToUnicodeEx() as X11 driver already does (try 2)

[Ken Thomases]

On Sep 10, 2013, at 5:45 AM, Phil Krylov wrote:

> On Tue, Sep 10, 2013 at 6:05 AM, Ken Thomases  wrote:
>> Is there a specific problem that you're trying to fix?
> 
> Yes. I didn't file a bug report yet, but an application I am using
> (IBM Translation Manager) has some functionality bound to Ctrl-[0-9/]
> keypresses. Now with the Mac driver when I press Ctrl-1, for example,
> the function bound to Ctrl-1 is executed, but also '1' gets inserted
> into the active Richedit control which is wrong.

Are you able to test what happens with IBM Translation Manager on Windows with 
some of the layouts I mentioned?


>> The Mac driver's ToUnicodeEx() implementation started out very similar to 
>> the X11 driver's.  It was basically a copy that I hacked on.  It originally 
>> had this same restriction.  However, some testing revealed that this is not 
>> an absolute restriction on Windows.  For example, in the Khmer keyboard 
>> layout, Control- produces characters for some digits.  Given that 
>> some Windows keyboard layouts produce characters for Control-, I 
>> figured it was best left up to the Mac keyboard layout.
>> 
>> Similarly for Control-.  For example, the Windows Chinese 
>> keyboard layouts produce characters for some of those keystrokes.  So does 
>> the Czech layout.
> 
> Yes I investigated the Mac keyboard layout files and found out that
> the real problem is there - Ctrl-1 is really bound to produce '1' etc.
> But when I press Ctrl-1 e.g. in TextEdit nothing gets inserted and a
> "beep" is heard, but there is no simple way to understand what's
> happening due to TextEdit's closed-sourcedness. Thanks for the Khmer
> and Czech Windows layouts info.

I'm not sure what's happening in TextEdit, either.  The source for TextEdit 
itself is actually available with the Xcode examples, but the problem is in 
Cocoa.  The NSEvent gives "1", etc. for the -characters and 
-charactersIgnoringModifiers.  The Control- special-case processing must 
happen within -[NSResponder -interpretKeyEvents:].  Indeed, in a test program, 
that calls back to -doCommandBySelector: with -noop: as the selector rather 
than calling -insertText: with a string.

That's not a result of the default Cocoa key binding in 
/System/Library/Frameworks/AppKit.framework/Versions/C/Resources/StandardKeyBinding.dict,
 which doesn't bind Control- at all.

For what it's worth, TextWrangler inserts digits for Control-.

Even so, perhaps Cocoa's refusal to insert text for Control- means it's 
OK to ignore that in the Mac driver.

Hmm.  In poking around with keyboard layouts, I've found that the Lithuanian 
one makes it very difficult to type digits without a numeric keypad.  One way 
is to use a dead key followed by one of the digit keys.  The other way is to 
use Control-Option-.  (Control- still doesn't do it, even though 
that would be very useful for this layout, which suggests the special handling 
in Cocoa is unconditional.)


>> If a modification like this were justified, it should go earlier in 
>> macdrv_ToUnicodeEx().  There's a section at the top where I moved the checks 
>> for combinations that don't produce characters.
>> Also, the check should test that Alt is _not_ pressed, since 
>> Control-Alt- and Control-Alt-Shfit- very 
>> commonly produce characters.  Control-Alt is a synonym for the AltGr key on 
>> some keyboards (e.g. Swiss).
>> 
>> Finally, if we're going to pursue this, there are some minor style issues 
>> I'd prefer be changed.  So, check with me before resubmitting.
> 
> Ok, I'll retry, although after this answer of yours I have rather
> little hope left as I don't know any other apps affected by this
> behaviour, so it can't be called a 'broad' breakage.

After considering what Cocoa is doing, I've changed my mind at least with 
respect to Control-.  I'd be willing to accept such a patch with the 
changes I described: put the check earlier in the function with the others; 
make sure Alt (VK_MENU) is not pressed; also make sure Option isn't pressed, 
which won't be reflected in lpKeyState but should be checked in 
thread_data->last_modifiers as is done when computing modifierKeyState.

The situation with Control- is less clear.  I'm not sure what to 
do there.  Cocoa doesn't seem to have as consistent special handling.  With 
some keyboard layouts, for some punctuation characters, Control- 
does insert text.  Therefore, some users may depend on that working in Wine.

As to my minor style quibbles:

* Don't refer to "Carbon" since it's not really accurate.  That comment 
probably won't be necessary as such in the new locat

Re: winemac: Don't return characters for Ctrl-(non-letter symbol) keypresses from ToUnicodeEx() as X11 driver already does (try 2)

[Ken Thomases]

Hi,

On Sep 9, 2013, at 7:38 PM, Phil Krylov wrote:

> Sorry, my first try had wrong patch attached
> 
> -- Ph.
> <0001-winemac-Don-t-return-characters-for-Ctrl-non-letter-try2.patch.txt>

Is there a specific problem that you're trying to fix?

The Mac driver's ToUnicodeEx() implementation started out very similar to the 
X11 driver's.  It was basically a copy that I hacked on.  It originally had 
this same restriction.  However, some testing revealed that this is not an 
absolute restriction on Windows.  For example, in the Khmer keyboard layout, 
Control- produces characters for some digits.  Given that some Windows 
keyboard layouts produce characters for Control-, I figured it was best 
left up to the Mac keyboard layout.

Similarly for Control-.  For example, the Windows Chinese keyboard 
layouts produce characters for some of those keystrokes.  So does the Czech 
layout.


It might still be possible to re-introduce this restriction if it can be 
justified based on broad breakage of apps.  Although even then, I'd want to 
make it possible to override the restriction with a registry setting or 
something.  You never know if the ability to enter characters using these key 
combinations might be crucial for certain Mac users because of the nature of 
their native keyboard layout.


If a modification like this were justified, it should go earlier in 
macdrv_ToUnicodeEx().  There's a section at the top where I moved the checks 
for combinations that don't produce characters.

Also, the check should test that Alt is _not_ pressed, since Control-Alt- and Control-Alt-Shfit- very commonly produce 
characters.  Control-Alt is a synonym for the AltGr key on some keyboards (e.g. 
Swiss).

Finally, if we're going to pursue this, there are some minor style issues I'd 
prefer be changed.  So, check with me before resubmitting.

Thanks and cheers,
Ken

Re: RFC: Mark dylib/mach-o on Mac OS X

[Ken Thomases]

Hi,

On Aug 27, 2013, at 4:08 PM, André Hentschel wrote:

> @@ -94,6 +95,9 @@ static void module_fill_module(const WCHAR* in, WCHAR* out, 
> size_t size)
> if (len > 3 && !strcmpiW(&out[len - 3], S_DotSoW) &&
> (l = match_ext(out, len - 3)))
> strcpyW(&out[len - l - 3], S_ElfW);
> +else if (len > 6 && !strcmpiW(&out[len - 6], S_DotDylibW) &&
> +(l = match_ext(out, len - 6)))
> +strcpyW(&out[len - l - 6], S_MachoW);

This isn't sufficient.  Even though the native dynamic library extension is 
.dylib on Mac OS X, Wine actually still uses .so as the extension for its 
libraries.  For example, kernel32.dll.so.

So, you have to change the earlier branch matching S_DotSoW, too.  Probably, it 
should just use "#ifdef __APPLE__" to pick the tag to apply, as 
module_get_type_by_name() does in a similar situation.

You do still need the test for .dylib for platform libraries.


> +dbg_printf("MACH-O\t");

I think it should be "Mach-O", not "MACH-O".

> +dbg_printf("MACH-O\t");

Same here.

I believe you missed a check for "" in symbols_info_cb() in symbol.c.  I'm 
not sure under what circumstances that would be important, though.

-Ken

Re: [PATCH] winemac.drv: Support the public UTF-16 type for Unicode text. (try 2)

[Ken Thomases]

On Aug 22, 2013, at 12:07 AM, Charles Davis wrote:

> Try 2: Eliminiate dead store. Sort export functions by name.

Looks OK to me.

-Ken

Re: [PATCH] winemac.drv: Support the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 21, 2013, at 11:49 PM, Charles Davis wrote:

> On Aug 21, 2013, at 10:34 PM, Ken Thomases wrote:
> 
>> On Aug 21, 2013, at 9:42 PM, Charles Davis wrote:
>>> +static HANDLE import_utf16_to_unicodetext(CFDataRef data)
>>> +{
>>> +const WCHAR *src;
>>> +unsigned long data_len;
>>> +unsigned long new_lines = 0;
>>> +LPWSTR dst;
>>> +unsigned long i, j;
>>> +HANDLE unicode_handle = NULL;
>> 
>> This is an unnecessary initialization / dead store, which is frowned upon.
> I was only blindly copying the code that you wrote ;).

Yeah, I know where it came from, but you then changed the code enough to make 
the initialization unnecessary where it had been necessary in the original. :)

-Ken

Re: [PATCH] winemac.drv: Support the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 21, 2013, at 9:42 PM, Charles Davis wrote:

> @@ -90,6 +93,9 @@ static CFDataRef export_hdrop_to_filenames(HANDLE data);
> static CFDataRef export_oemtext_to_utf8(HANDLE data);
> static CFDataRef export_text_to_utf8(HANDLE data);
> static CFDataRef export_unicodetext_to_utf8(HANDLE data);
> +static CFDataRef export_oemtext_to_utf16(HANDLE data);
> +static CFDataRef export_text_to_utf16(HANDLE data);
> +static CFDataRef export_unicodetext_to_utf16(HANDLE data);

Please keep the export_* functions sorted (although I'm fine with utf8 coming 
before utf16).  This applies to the function definitions, too.  (Yes, this is 
an exceedingly minor issue.  Sorry.)


> +static HANDLE import_utf16_to_unicodetext(CFDataRef data)
> +{
> +const WCHAR *src;
> +unsigned long data_len;
> +unsigned long new_lines = 0;
> +LPWSTR dst;
> +unsigned long i, j;
> +HANDLE unicode_handle = NULL;

This is an unnecessary initialization / dead store, which is frowned upon.

Thanks!

-Ken

Re: [PATCH] winemac.drv: Use the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 21, 2013, at 10:12 AM, Ken Thomases wrote:

> On Aug 20, 2013, at 10:49 PM, Charles Davis wrote:
> 
>> In the Windows world, "Unicode" almost universally means "UTF-16". So,
>> use the well-known UTF-16 type instead of making up our own.
>> 
>> I have to wonder if there was a good reason Ken didn't use this
>> initially.
> 
> Please hold this patch while I review it.
> 
> I think there is a good reason why I didn't use it, but I have to figure it 
> out again. :-/
> 
> It has to do with a promised pasteboard type (what Microsoft calls delayed 
> rendering) and the conversions to the other text types. I think we need to 
> use a custom type to distinguish between being asked for promised data vs. 
> being asked for a conversion.

Well, that wasn't the reason.  However, I've found a simpler reason not to 
commit this.  The data supplied for CF_UNICODETEXT is null-terminated, but 
public.utf16-plain-text shouldn't be.  (Neither CFString nor NSString treat a 
0x code unit specially.  It just ends up part of the string.)

Was there a specific compatibility problem you were trying to solve?  Or just 
reviewing the code and found this strange and wanted to improve it?

If you like, it may make sense to add conversions to public.utf16-plain-text 
like those done for public.utf8-plain-text, but the import/export functions 
will have to add/remove the terminating null.

-Ken

Re: [PATCH] winemac.drv: Use the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 20, 2013, at 10:49 PM, Charles Davis wrote:

> In the Windows world, "Unicode" almost universally means "UTF-16". So,
> use the well-known UTF-16 type instead of making up our own.
> 
> I have to wonder if there was a good reason Ken didn't use this
> initially.

Please hold this patch while I review it.

I think there is a good reason why I didn't use it, but I have to figure it out 
again. :-/

It has to do with a promised pasteboard type (what Microsoft calls delayed 
rendering) and the conversions to the other text types. I think we need to use 
a custom type to distinguish between being asked for promised data vs. being 
asked for a conversion.

-Ken

Re: configure.ac: allow disabling winemac.drv

[Ken Thomases]

On Aug 8, 2013, at 5:56 PM, Austin English wrote:

> +AC_ARG_WITH(winemac,   AS_HELP_STRING([--without-winemac],[do not build 
> native Mac (Cocoa) driver]),
> +[if test "x$withval" = "xno"; then 
> ac_cv_header_ApplicationServices_ApplicationServices_h=no; fi])

A switch to disable the Mac driver is fine with me, but you can't pretend that 
the ApplicationServices framework header isn't present.  There are other places 
within Wine which check that.
http://source.winehq.org/source/dlls/windowscodecs/icnsformat.c#L24
http://source.winehq.org/source/dlls/winspool.drv/info.c#L48

-Ken

Re: gdi32: Avoid a crash by loading AppKit before using Core Text if might use the Mac driver.

[Ken Thomases]

On Aug 2, 2013, at 9:57 AM, Ken Thomases wrote:

> OK.  So, is it acceptable to load AppKit regardless of which graphics driver 
> is configured?

And, if so, would you prefer that I just link gdi32 against AppKit rather than 
loading it dynamically?  That would mean that it would be loaded even for 
processes which wouldn't enumerate fonts.

-Ken

Re: gdi32: Avoid a crash by loading AppKit before using Core Text if might use the Mac driver.

[Ken Thomases]

On Aug 2, 2013, at 9:19 AM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> On Aug 2, 2013, at 4:22 AM, Alexandre Julliard wrote:
>>> That's ugly. freetype.c has no business knowing about the details of the
>>> graphics driver.
>> 
>> Is it acceptable to load the graphics driver at that point, by calling 
>> DRIVER_load_driver("display", …)?
> 
> That code is called during dll attach, so no, that won't work right.

OK.  So, is it acceptable to load AppKit regardless of which graphics driver is 
configured?

-Ken

Re: gdi32: Avoid a crash by loading AppKit before using Core Text if might use the Mac driver.

[Ken Thomases]

On Aug 2, 2013, at 4:22 AM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> +/* If the Mac driver might be used, then load AppKit now before using 
>> the Core Text API.
>> +   Otherwise, AppKit crashes on Mac OS X 10.7+ because CTFontDescriptor 
>> isn't toll-free
>> +   bridged to NSCTFontDescriptor.  That bridging is only set up if 
>> AppKit has been loaded
>> +   when CTFontDescriptor is first used. */
>> +/* @@ Wine registry key: HKCU\Software\Wine\Drivers */
>> +if (!RegOpenKeyA(HKEY_CURRENT_USER, "Software\\Wine\\Drivers", &hkey))
>> +{
>> +char buffer[MAX_PATH];
>> +DWORD type, count = sizeof(buffer);
>> +
>> +if (!RegQueryValueExA(hkey, "Graphics", 0, &type, (LPBYTE) buffer, 
>> &count))
>> +{
>> +char *name = buffer;
>> +
>> +usingMacDriver = FALSE;
>> +while (name)
>> +{
>> +char *next = strchr(name, ',');
>> +if (next) *next++ = 0;
>> +
>> +if (!strcmp(name, "mac"))
>> +{
>> +usingMacDriver = TRUE;
>> +break;
>> +}
>> +
>> +name = next;
>> +}
>> +}
>> +
>> +RegCloseKey(hkey);
>> +}
> 
> That's ugly. freetype.c has no business knowing about the details of the
> graphics driver.

Is it acceptable to load the graphics driver at that point, by calling 
DRIVER_load_driver("display", …)?

-Ken

Re: Need help with debugging a directx9 game crashing

[Ken Thomases]

On Jul 25, 2013, at 2:11 PM, Qian Hong wrote:

> On Fri, Jul 26, 2013 at 2:52 AM, Ken Thomases  wrote:
>> I think your Valgrind results are telling us that there's a bug in the game 
>> where it's using an uninitialized stack variable.
>> 
>> There's still a chance that it's something in Wine that's using the 
>> uninitialized variable and passing a garbage value to the game, causing it 
>> to crash.  I'm not sure if Valgrind would attribute that to Wine for copying 
>> the uninitialized variable's value or to the game for using what Wine gave 
>> it.  I _think_ Valgrind would blame Wine in that case.  Since it's blaming 
>> the game, I lean toward the bug being in the game.
>> 
>> There may not be any solution other than reporting the issue to the game 
>> developer.
> 
> Hmm, thanks a lot for the great analysis :)
> I've just sent a message to someone in the game company, not sure if
> they care :)

I suppose another possibility is that Wine is returning a value to the game 
which is causing it to take a code path it never takes on Windows and it's only 
this code path which results in the game reading the uninitialized variable.  
In that case, there's still a game bug, but we may be able to work around it by 
changing Wine.

It's going to be really hard to figure out, though, without cooperation from 
the game developers.

-Ken

Re: Need help with debugging a directx9 game crashing

[Ken Thomases]

Hi Qian,

On Jul 25, 2013, at 11:53 AM, Qian Hong wrote:

> On Thu, Jul 25, 2013 at 2:33 AM, Ken Thomases  wrote:
>> Hmm.  You may need to mark the dummy array as volatile to prevent it from 
>> being optimized away.  Worth trying.  You might also increase the size of 
>> the array.
> 
> You are right, thanks! Increasing the size of the array helps, the
> minimum working size I found is 588, I've attach the patch and the
> backtrace to Bug 34125.
> 
> In my hack, I let:
> +char dummy[588];
> +memset(dummy, 0x55, sizeof(dummy));
> +dummy[4] = 0x88;
> +dummy[5] = 0x77;
> +dummy[6] = 0x66;
> +dummy[7] = 0x55;
> /* why 584 doesn't work? */
> 
> And I get a crashing like:
> Unhandled exception: page fault on read access to 0x55667788 in 32-bit code

I think your Valgrind results are telling us that there's a bug in the game 
where it's using an uninitialized stack variable.

There's still a chance that it's something in Wine that's using the 
uninitialized variable and passing a garbage value to the game, causing it to 
crash.  I'm not sure if Valgrind would attribute that to Wine for copying the 
uninitialized variable's value or to the game for using what Wine gave it.  I 
_think_ Valgrind would blame Wine in that case.  Since it's blaming the game, I 
lean toward the bug being in the game.

There may not be any solution other than reporting the issue to the game 
developer.

-Ken

Re: [PATCH] winemac.drv: Advertise some legacy WGL extensions in the GL_EXTENSIONS string. (try 3)

[Ken Thomases]

On Jul 25, 2013, at 10:49 AM, Charles Davis wrote:

> Try 3: Don't advertise WGL_EXT_swap_control if vsync is disabled.

Looks good.

Thanks,
Ken

Re: [PATCH] winemac.drv: Advertise some legacy WGL extensions in the GL_EXTENSIONS string. (try 2)

[Ken Thomases]

On Jul 25, 2013, at 9:57 AM, Ken Thomases wrote:

> If vsync is disallowed, the corresponding functions are left as NULL in the 
> OpenGL function table, so clients may crash if WGL_EXT_swap_control is 
> advertised.

Well, this part is wrong.  They won't crash because opengl32 protects against 
that, but still we shouldn't advertise it if support for it is disabled.

-Ken

Re: [PATCH] winemac.drv: Advertise some legacy WGL extensions in the GL_EXTENSIONS string. (try 2)

[Ken Thomases]

On Jul 24, 2013, at 9:41 PM, Ken Thomases wrote:

> On Jul 24, 2013, at 9:34 PM, Charles Davis wrote:
> 
>> Try 2: Don't duplicate a constant string; we might need to add more legacy
>>  extensions. (Hopefully not. ;)
> 
> Thanks for making that change.  Looks good to me.

Actually, on further thought I realize that we shouldn't be advertising 
WGL_EXT_swap_control unconditionally.  It should be controlled by the 
AllowVerticalSync registry setting / allow_vsync variable.  See 
load_extensions().  If vsync is disallowed, the corresponding functions are 
left as NULL in the OpenGL function table, so clients may crash if 
WGL_EXT_swap_control is advertised.

Regards,
Ken

Re: [PATCH] winemac.drv: Advertise some legacy WGL extensions in the GL_EXTENSIONS string. (try 2)

[Ken Thomases]

On Jul 24, 2013, at 9:34 PM, Charles Davis wrote:

> Try 2: Don't duplicate a constant string; we might need to add more legacy
>   extensions. (Hopefully not. ;)

Thanks for making that change.  Looks good to me.

Cheers,
Ken

Re: [PATCH] winemac.drv: Also return the number of pixel formats if the caller didn't supply a PIXELFORMATDESCRIPTOR.

[Ken Thomases]

On Jul 23, 2013, at 10:26 PM, Charles Davis wrote:

> This is akin to what glxdrv already does, and apparently what Windows
> does. It allows the GLView sample from the OpenGL SuperBible to
> correctly enumerate pixel formats with the Mac driver.
> ---
> dlls/winemac.drv/opengl.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/dlls/winemac.drv/opengl.c b/dlls/winemac.drv/opengl.c
> index 0922371..fb294d1 100644
> --- a/dlls/winemac.drv/opengl.c
> +++ b/dlls/winemac.drv/opengl.c
> @@ -3281,7 +3281,7 @@ int macdrv_wglDescribePixelFormat(HDC hdc, int fmt, 
> UINT size, PIXELFORMATDESCRI
> 
> TRACE("hdc %p fmt %d size %u descr %p\n", hdc, fmt, size, descr);
> 
> -if (fmt <= 0 || fmt > ret) return ret;
> +if (fmt <= 0 || fmt > ret || !descr) return ret;
> if (size < sizeof(*descr)) return 0;
> 
> pf = &pixel_formats[fmt - 1];
> -- 
> 1.7.5.4

Looks good to me.  Thanks!

-Ken

Re: [PATCH] winemac.drv: Advertise some legacy WGL extensions in the GL_EXTENSIONS string.

[Ken Thomases]

Hi,

On Jul 23, 2013, at 10:26 PM, Charles Davis wrote:

> @@ -1226,8 +1227,10 @@ static BOOL init_gl_info(void)
> gl_info.glVersion = HeapAlloc(GetProcessHeap(), 0, strlen(str) + 1);
> strcpy(gl_info.glVersion, str);
> str = (const char*)opengl_funcs.gl.p_glGetString(GL_EXTENSIONS);
> -gl_info.glExtensions = HeapAlloc(GetProcessHeap(), 0, strlen(str) + 1);
> +gl_info.glExtensions = HeapAlloc(GetProcessHeap(), 0, strlen(str) +
> +sizeof(" WGL_EXT_extensions_string WGL_EXT_swap_control"));
> strcpy(gl_info.glExtensions, str);
> +strcat(gl_info.glExtensions, " WGL_EXT_extensions_string 
> WGL_EXT_swap_control");

As a minor nitpick, I would prefer to avoid that duplicated string literal.

Otherwise, looks good to me.

Cheers,
Ken

Re: Need help with debugging a directx9 game crashing

[Ken Thomases]

On Jul 24, 2013, at 12:49 PM, Qian Hong wrote:

> Hi Ken,
> 
> On Sun, Jul 21, 2013 at 2:39 PM, Ken Thomases  wrote:
>> Instead of enabling the trace, try replacing it with something like:
>> 
>>char dummy[256];
>>memset(dummy, 0x55, sizeof(dummy));
>> 
>> I bet the crash will change from a read access to 0x0001 to 0x.
>> 
> 
> Thanks for the advice, it sounds cool and I learned new things, thank you.
> However, it doesn't help in this case, I didn't found any change after
> the dummy hack.

Hmm.  You may need to mark the dummy array as volatile to prevent it from being 
optimized away.  Worth trying.  You might also increase the size of the array.

-Ken

Re: Need help with debugging a directx9 game crashing

[Ken Thomases]

On Jul 19, 2013, at 8:34 AM, Qian Hong wrote:

> I was debugging on a popular game [1] [2] (9 GB), it crashes on start.
> 
> +relay,+seh,+tid log show that there are some calls to
> wined3d.wined3d_mutex_lock / wined3d.wined3d_buffer_map /
> wined3d.wined3d_mutex_unlock before crashing, so I turn on +d3d9 trace
> to get a new log in details.
> 
> To my surprise, the game doesn't crash anymore with +d3d9 trace!
> 
> After a serials of bisect, I found d3d9_AddRef is the key function
> call related to the crashing. If I remove the TRACE statement in line
> 71, the game will crash with +d3d9 as well; if I upgrade the TRACE in
> 71 to FIXME, the game will not crash even without +d3d9.
> 
> 66 static ULONG WINAPI d3d9_AddRef(IDirect3D9Ex *iface)
> 67 {
> 68 struct d3d9 *d3d9 = impl_from_IDirect3D9Ex(iface);
> 69 ULONG refcount = InterlockedIncrement(&d3d9->refcount);
> 70
> 71 TRACE("%p increasing refcount to %u.\n", iface, refcount);
> 72
> 73 return refcount;
> 74 }
> 
> Further tests show that the simplest hack to avoid crashing is
> replacing line 71 to:
> FIXME("anything %x\n", 0xdeadbeef);
> 
> I have no idea what the real fix is, any suggestion what is the next
> step to debug?

> Backtrace on crash:
> Wine-dbg>c
> Unhandled exception: page fault on read access to 0x0001 in 32-bit
> code (0x0649e6e9).

This reminds me of bugs where programs fail to initialize variables before use. 
 So, the variable contains whatever value happened to be left at that stack 
location by previous calls.  The specific behavior of those previous calls can 
leave different values, so changing seemingly unrelated code changes the crash 
(or makes it go away).

Instead of enabling the trace, try replacing it with something like:

char dummy[256];
memset(dummy, 0x55, sizeof(dummy));

I bet the crash will change from a read access to 0x0001 to 0x.

The question is: is it a bug in the game or a bug in Wine where Wine is 
returning a garbage pointer to the game and the game is innocently 
dereferencing it?  For that, you'd have to trace the execution from the call to 
d3d9_AddRef() to the crashing point to see what else is called and whether it 
uses an uninitialized value.

Of course, another approach would be to run under Valgrind, if you can get it 
to work.

-Ken

Re: Fix to support input of characters with diacritic symbols using AltGr

[Ken Thomases]

On Jul 1, 2013, at 4:26 PM, Ken Thomases wrote:

> On Jul 1, 2013, at 3:29 PM, Юрий Воротилов wrote:
> 
>> Fix to support input of characters with diacritic symbols using AltGr (e.g. 
>> Latvian - AltGr+a=ā).
>> Tested on Ubuntu 12.04 and Centos 6.
>> I also have a bug opened in Crossover tracker:
>> http://www.codeweavers.com/support/tickets/browse/?ticket_id=935141
>> 
>> ---
>> dlls/winex11.drv/keyboard.c |   45 
>> +--
>> include/winuser.h   |1 +
>> server/queue.c  |5 -
>> 3 files changed, 31 insertions(+), 20 deletions(-)
>> <0001-Support-input-of-characters-with-diacritic-symbols-usi.txt>
> 
> I don't believe this is the correct approach.  You don't want to be modifying 
> the server and you can't arbitrarily add a new KEYEVENTF_* value.  You can't 
> assume that applications won't leave junk in the non-standard bits of the 
> flags when they call SendInput() or keybd_event().
> 
> This needs to be implemented entirely within the X11 driver.  It needs to 
> properly translate your X11 keyboard layout into Win32 behavior.  If the 
> problem is that AltGr is mapping to VK_MENU, and thus causing key events to 
> generate WM_SYSKEYDOWN/UP and not go through translation, then you need to 
> make AltGr not map to VK_MENU.  Although you really need to test that on 
> Windows.  My understanding is that AltGr appears to Windows apps as 
> Alt+Control.
> 
> In any case, I know that, on Mac OS X, the Option key is used similarly to 
> AltGr.  It accesses additional characters from the keyboard layout.  This 
> works when Option is configured in the X keymap to generate XK_Mode_switch.  
> It requires no changes to Wine.  So perhaps the right approach is to make 
> XK_ISO_Level3_Shift and XK_ISO_Group_Shift behave the same way as 
> XK_Mode_switch.  (You could also just change your X keymap to actually map 
> the key to XK_Mode_switch.)

Oh, and I forgot to mention: please don't rename variables unnecessarily 
(AltGrMask -> KeyEventModifiers), make whitespace changes on lines you're not 
otherwise modifying (stuff in X11DRV_InitKeyboard()), or change trace formats 
arbitrarily (%x -> %X).

-Ken

Re: Fix to support input of characters with diacritic symbols using AltGr

[Ken Thomases]

On Jul 1, 2013, at 3:29 PM, Юрий Воротилов wrote:

> Fix to support input of characters with diacritic symbols using AltGr (e.g. 
> Latvian - AltGr+a=ā).
> Tested on Ubuntu 12.04 and Centos 6.
> I also have a bug opened in Crossover tracker:
> http://www.codeweavers.com/support/tickets/browse/?ticket_id=935141
> 
> ---
>  dlls/winex11.drv/keyboard.c |   45 
> +--
>  include/winuser.h   |1 +
>  server/queue.c  |5 -
>  3 files changed, 31 insertions(+), 20 deletions(-)
> <0001-Support-input-of-characters-with-diacritic-symbols-usi.txt>

I don't believe this is the correct approach.  You don't want to be modifying 
the server and you can't arbitrarily add a new KEYEVENTF_* value.  You can't 
assume that applications won't leave junk in the non-standard bits of the flags 
when they call SendInput() or keybd_event().

This needs to be implemented entirely within the X11 driver.  It needs to 
properly translate your X11 keyboard layout into Win32 behavior.  If the 
problem is that AltGr is mapping to VK_MENU, and thus causing key events to 
generate WM_SYSKEYDOWN/UP and not go through translation, then you need to make 
AltGr not map to VK_MENU.  Although you really need to test that on Windows.  
My understanding is that AltGr appears to Windows apps as Alt+Control.

In any case, I know that, on Mac OS X, the Option key is used similarly to 
AltGr.  It accesses additional characters from the keyboard layout.  This works 
when Option is configured in the X keymap to generate XK_Mode_switch.  It 
requires no changes to Wine.  So perhaps the right approach is to make 
XK_ISO_Level3_Shift and XK_ISO_Group_Shift behave the same way as 
XK_Mode_switch.  (You could also just change your X keymap to actually map the 
key to XK_Mode_switch.)

-Ken

Re: comctl32: Use default GUI font as a fallback instead of a (non-existent) Arial.

[Ken Thomases]

On Jun 18, 2013, at 1:26 AM, Dmitry Timoshkov wrote:

> Daniel Jeliński  wrote:
> 
>> I'm quite sure you didn't mean to ignore return value of GetStockObject and
>> leave logfont uninitialized here.
> 
> GetStockObject(DEFAULT_GUI_FONT) can't fail.

I think the question is: what does line 1917 do?
http://source.winehq.org/git/wine.git/blob/cc086f09ae46c77e6eb4a817ae567294d3f795ec:/dlls/comctl32/tab.c#l1917

Does GetStockObject(DEFAULT_GUI_FONT) have some sort of side effect that's not 
obvious?

-Ken

Re: Using of mac driver completely breaks tablets support?

[Ken Thomases]

Hi,

On Jun 12, 2013, at 10:47 AM, Andrey Upadyshev wrote:

> I've installed a Wine 1.5.31 instead of 1.4.smthng and found that tablets
> support is broken. Some research gives me that wintab32 DLL fails to init
> because winemac driver doesn't implement tablet related functions
> (LoadTabletInfo etc) unlike x11 driver.
> 
> I'm a Wine newcomer so probably I get something wrong about it...

No, you're correct.  The Mac driver does not yet support tablet input devices.


> How can I switch back to x11 driver?

In the registry, set the following:

[HKEY_CURRENT_USER\Software\Wine\Drivers]
"Graphics"="x11"

Some of the basics of how to do that (although not this specific thing) are 
documented at .

Regards,
Ken

Re: [PATCH 1/8] winemac: Don't unminimize a window on first activation during start-up.

[Ken Thomases]

On Jun 4, 2013, at 5:15 AM, Dmitry Timoshkov wrote:

> Ken Thomases  wrote:
> 
>> ---
>> dlls/winemac.drv/cocoa_app.h |2 ++
>> dlls/winemac.drv/cocoa_app.m |3 ++-
>> 2 files changed, 4 insertions(+), 1 deletions(-)
> 
> I wonder, do you actually run 'make test' in dlls/user32/tests with
> your changes? My guess is that you don't.

In fact, I spent many hours today doing just that.  And some of these patches 
were to fix specific test failures.

-Ken

Re: (try 2)macdrv: rework the way we handle cursor position and composition text

[Ken Thomases]

On May 29, 2013, at 1:11 PM, Aric Stewart wrote:

> ---
> dlls/winemac.drv/cocoa_window.m |  9 +
> dlls/winemac.drv/event.c|  5 ---
> dlls/winemac.drv/ime.c  | 85 +
> dlls/winemac.drv/macdrv.h   |  1 -
> dlls/winemac.drv/macdrv_cocoa.h |  6 +--
> 5 files changed, 29 insertions(+), 77 deletions(-)
> 
> 
> <0014-macdrv-rework-the-way-we-handle-cursor-position-and-co.txt>

Looks good to me.

-Ken

Re: macdrv: rework the way we handle cursor position and composition text

[Ken Thomases]

On May 29, 2013, at 10:37 AM, Aric Stewart wrote:

> ---
> dlls/winemac.drv/cocoa_window.m |  9 +
> dlls/winemac.drv/event.c|  3 --
> dlls/winemac.drv/ime.c  | 85 +
> dlls/winemac.drv/macdrv.h   |  1 -
> dlls/winemac.drv/macdrv_cocoa.h |  5 +--
> 5 files changed, 29 insertions(+), 74 deletions(-)

I know I reviewed this several times, but I failed to notice until now: you 
didn't remove the definition of IM_SET_CURSOR_POS in the enum and the entry in 
the table in dbgstr_event().

-Ken

Re: [PATCH 2/2] user32/tests: Test that sending WM_CANCELMODE cancels tracking in window's menu bar.

[Ken Thomases]

On May 17, 2013, at 12:21 AM, Dmitry Timoshkov wrote:

> Ken Thomases  wrote:
> 
>> dlls/user32/tests/menu.c |   21 -
> 
> Please add a message test instead, it would much more clearer show what is
> going on.

I added to the existing tests of WM_CANCELMODE because that's what this new 
test is.  It's just like the existing tests except with a different way of 
initiating menu tracking.


>> +{MSG msg;   while (PeekMessage(&msg, 0, 0, 0, PM_REMOVE)) 
>> DispatchMessage(&msg);}
> 
> Please avoid creating such embedded blocks.

Again, that's just directly copied from the earlier code in the same test 
function.  I suppose I could break that up, if you prefer.

-Ken

Re: winemac.drv: add fullscreen mode support for Mac OS X 10.7+

[Ken Thomases]

On May 15, 2013, at 8:49 PM, Kevin Eaves wrote:

> …  The title is also empty when exiting borderless to titled, which can be 
> fixed here.

Hmm, good catch.  I'll have to check, but I suspect the window no longer 
remembers its title at that point.  And the Mac driver doesn't currently store 
it anywhere else.  So, we may need to do so in order to restore it.


>> Also, if the Windows program changes a window from resizable to 
>> non-resizable, then we should probably pull the window out of Cocoa 
>> fullscreen.  The Windows program _may_ have intended to make the window 
>> full-screen but we can't know that.  It may simply be changing the window 
>> style for some other purpose, and that may make it no longer a candidate for 
>> Cocoa full-screen mode.
> 
> Allowing Windows fullscreen in Cocoa fullscreen was never a very good idea.  
> I did not want to force the window out, but a borderless window would not be 
> allowed to enter Cocoa fullscreen, so it probably should not be there.

Yeah.  It may make for a poor user experience to force the window out of Cocoa 
full-screen, especially if it's just going to go Windows full-screen right 
afterward, but I think it's probably necessary.  I think it's a relatively rare 
occurrence, anyway.


>> I disagree.  Windows programs that are not expecting their window to resize 
>> (because they haven't made them resizable) can misbehave badly if the window 
>> does resize.  Just because that didn't happen in your testing doesn't mean 
>> it won't.
>> 
>> As you mentioned in your original email, Cocoa full-screen is roughly akin 
>> to window resizing.  So, it should be disabled if/when the window is made 
>> non-resizable.
> 
> I do not see the reason to disable buttons and make the windows 
> non-resizable.  Probably wrong here, but doesn't this just eliminate 
> click-through, which is typical of Windows apps, because the Windows paradigm 
> stinks?

Well, that's how it works on Windows.  And the X11 driver in Wine.  And native 
Mac apps.  (Open TextEdit.  Its window has all the buttons enabled and can be 
resized.  Open the modal Open dialog.  The document window still has all its 
buttons and, on 10.6, the grow box, but they're disabled.  On 10.7+, the resize 
cursors around the edge still show, but they're ineffective.  I suppose we can 
achieve that same effect by not using the min and max sizes to prevent resize 
but do it by overriding -windowWillResize:toSize:, but I prefer it to be 
obviously not resizable.)

The main reason is that some Windows apps will probably freak out if their 
windows get resized, closed, or minimized when they're supposed to be disabled.


>> I don't follow this.  It is possible to resize any window even non-resizable 
>> ones?  To what are you referring?  Possible how?  Do you mean specifically 
>> just with this full-screen patch?
> 
> It is possible to do anything we want with a window.  The actual Windows 
> program not freaking out is another story.  Just pointing out that we are not 
> limited.

Sure, but Wine is aiming for compatibility with Windows apps, so the "not 
freaking out" thing is kind of a big deal. ;)

-Ken

Re: winemac: Disable the zoom button for windows that are disabled.

[Ken Thomases]

I have submitted an alternative patch to fix this issue.  It ended up being 
more involved.

First, on considering exactly how disabling a window should interact with its 
resizability features, I decided that disabling should not remove 
NSResizableWindowMask from the style mask.

Consider a window whose style mask is just NSTitledWindowMask | 
NSResizableWindowMask – that is, it's not closable or minimizable.  If 
disabling it removed NSResizableWindowMask, then it would have no bits for the 
window buttons in the mask.  So, the window would not display any buttons.  
That's not the correct behavior.  Disabling should leave all the same buttons 
visible, it's just that they should be, well, disabled.

Similarly for the resize indicator (a.k.a. grow box).  The Mac driver currently 
hides it, but I may make that an option in the future.  Anyway, if the window 
is resizable and the resize indicator is showing, then disabling it should not 
hide it, just make it unresponsive to clicks and drags.  (That's how a 
Mac-native app behaves when a resizable window is behind a modal dialog, for 
example.)

So, to disable the zoom button, I just access it directly and disable it as you 
did in your patches.  However, we also need to disable the resize cursors at 
the window edges.  So, I set the minimum and maximum sizes of the window to 
match the current size.  (The resize cursors themselves don't show for non-main 
windows, but such windows can still be resized by Command-dragging their edges. 
 That  would work with regedit when its Import dialog is open had I not set the 
min and max sizes, which would be bad.)


I encountered some difficulties in implementing the above strategy, however.  
Basically, -[NSWindow setStyleMask:] is pretty broken on 10.7+.  It works 
properly on 10.6.

This isn't related to disabling windows so much as to the Windows program 
changing the style such that the window changes its resizability.

It turns out that adding or removing NSResizableWindowMask when neither of the 
other two buttons are present in the style mask does _not_ show or hide the 
buttons like it should.  So, if you start with just NSTitledWindowMask and add 
NSResizableWindowMask, the window still shows no buttons.  And if you start 
with NSTitledWindowMask | NSResizableWindowMask and remove 
NSResizableWindowMask, then the window does not hide the buttons.

The workaround is to also toggle NSClosableWindowMask temporarily in that 
operation.  That seems to force Cocoa to refresh its notion of whether the 
buttons should show or not.

-Ken

Re: winemac: Disable the zoom button for windows that are disabled.

[Ken Thomases]

On May 12, 2013, at 3:22 PM, Ken Thomases wrote:

> As mentioned in the other thread, I'm not seeing the problem that this is 
> meant to solve on 10.6.  I will test 10.7+ tomorrow.

OK, I've tested on 10.8 and I see that the zoom button highlights when the 
mouse moves over it, although clicking on it does nothing.

I've also verified with a simple Cocoa test app.  Toggling 
NSResizableWindowMask in the style mask does not seem to affect the zoom 
button.  It works the same in the other direction, too.  If the window starts 
without NSResizableWindowMask in its style mask, then adding it in later 
doesn't enable the zoom button.  I've got some more tests I want to run.

Oddly, the zoom button behaves properly on 10.8 with the Mac driver in 
CrossOver 12.x.  That's because I handled disabled windows somewhat similarly 
to what your patch does, disabling the zoom button explicitly.  However, I also 
disable the resize cursors at the edge by setting the max and min size for the 
window to the current frame size.  We may need to go that route.  I changed it 
for a reason, though, when submitting the Mac driver to Wine.  I wish I could 
remember why. :-/

-Ken

Re: winemac: Disable the zoom button for windows that are disabled.

[Ken Thomases]

Hi Kevin,

As mentioned in the other thread, I'm not seeing the problem that this is meant 
to solve on 10.6.  I will test 10.7+ tomorrow. Also, I'm pretty sure that 
setting the style mask after disabling the buttons can reenable them.  At least 
in some versions of the frameworks, we've seen that -setStyleMask: basically 
resets the standard buttons from scratch.  Indeed, in my testing, this patch 
reenables the close and minimize buttons on a disabled window.

Regards,
Ken

Re: winemac.drv: add fullscreen mode support for Mac OS X 10.7+

[Ken Thomases]

Hi Kevin,

I'm bringing this back to wine-devel.  Please CC the list unless you intend to 
take things private, in which case please say so.

On May 10, 2013, at 6:48 PM, Kevin Eaves wrote:

> I should have made everything clear, sorry.  It was for personal use, so it's 
> pretty nasty, but works great.  The only real problem is with user32.  
> Everything else is easy to clean up and get working with Wine properly 
> without any issues.  Just wanted to show that.

Sure, that's what I figured.


> On May 9, 2013, at 9:56 PM, Ken Thomases wrote:
> 
>>> The fullscreen button appears if a main window is resizable.
>> 
>> We may want to narrow this down a bit.  For example, the common file dialogs 
>> are resizable, but probably shouldn't be full-screen-enabled.  Perhaps no 
>> owned window should be.
> 
> Only windows in the windows cycle should be fullscreen enabled.  If the 
> window can be manually moved to its own space, then it should be able to go 
> fullscreen.  No dialogs are enabled.  These follow the main window to spaces. 
>  If in Cocoa fullscreen, the new window automatically opens fullscreen in its 
> own space.  Everything works just like Mac applications with this setting.

Ah, I failed to notice before that the use of 
NSWindowCollectionBehaviorFullScreenPrimary was only in the same code path as 
NSWindowCollectionBehaviorParticipatesInCycle.  Yes, that makes sense.

>> From cocoa_window.m.diff:
>> 
>>> +#import 
>> 
>> 
>> Was that actually needed?  cocoa_window.h already imports .
>> 
>> ...
>> 
>>> +if (style & NSResizableWindowMask)
>>> +[[self standardWindowButton:NSWindowZoomButton] 
>>> setEnabled:!self.disabled];
> 
> These should have not been included in the diff.  It's unrelated to Cocoa 
> fullscreen, however, the zoom button is currently bugged.  It is not disabled 
> when the window is disabled.  To reproduce the problem: launch regedit, open 
> the import window, and click the zoom button on the main window.  I sent a 
> patch for it.

I'm not seeing that problem happen here.  Mind you, I'm currently testing on 
10.6.  I'll have to run some tests on 10.7+ tomorrow.

I find it improbable that removing NSResizableWindowMask from the style doesn't 
disable the zoom button.  That style is the only thing which enables the zoom 
button to begin with.  In other words, in a normal Mac app, one would not need 
to disable the zoom button separately.


>>> -if (captured || fullscreen)
>>> +if (captured || (fullscreen && !(normalStyleMask & 
>>> NSResizableWindowMask)))
>> 
>> You're trying to distinguish between a window that was full-screened at the 
>> instigation of the Windows program versus one which has been full-screened 
>> by Cocoa, right?  Hmm.  I think we need a better technique to determine 
>> that.  I agree that it's not likely the Windows program would make its 
>> window full-screen while still allowing it to be resized but it feels wrong 
>> to assume that.
>> 
>> Also, I think we can entirely avoid setting the window level for windows 
>> that have been Cocoa full-screened.  Setting the window level is mostly 
>> about relationship to other windows, but a Cocoa full-screen window should 
>> be on a space by itself, shouldn't it?
>> 
>>> +else if ([self styleMask] & NSFullScreenWindowMask)
>>> +level = NSMainMenuWindowLevel + 2;
>> 
>> I'm not following why Cocoa full-screened windows should be at +2 vs. +1.
> 
> The window levels are automatically adjusted and the menu is in a hover 
> state.  We need to prevent the menu from being hidden, and hide the menu if 
> the Windows program goes fullscreen while in Cocoa fullscreen (+1 doesn't cut 
> it).

I'm going to have to do some testing to understand this, too.  
NSMainMenuWindowLevel should be in the same level as the menu (meaning could be 
above or below due to normal window ordering operations).  
NSMainMenuWindowLevel + 1 should always be above the menu, regardless.  
NSMainMenuWindowLevel + 2 should not be necessary.  But maybe something weird 
is going on.

Also, if the Windows program changes a window from resizable to non-resizable, 
then we should probably pull the window out of Cocoa fullscreen.  The Windows 
program _may_ have intended to make the window full-screen but we can't know 
that.  It may simply be changing the window style for some other purpose, and 
that may make it no longer a candidate for Cocoa full-screen mode.


>>> +- (NSSize)window:(NSWindow *)window 
>>> willUseFullScreenContentSize:(NSSize)p

Re: winemac.drv: add fullscreen mode support for Mac OS X 10.7+

[Ken Thomases]

Hi,

On May 9, 2013, at 5:48 PM, Kevin Eaves wrote:

> Attached patch is a proof-of-concept for the fullscreen APIs provided in OS X 
> 10.7+, which allows applications to enter fullscreen in a separate desktop 
> space.

Thanks for this.  It's quite interesting.

> The fullscreen button appears if a main window is resizable.

We may want to narrow this down a bit.  For example, the common file dialogs 
are resizable, but probably shouldn't be full-screen-enabled.  Perhaps no owned 
window should be.

> Basically this feature is the zoom button on crack.  It doesn't actually go 
> fullscreen 'natively' but simply resizes the window fullscreen, and 
> automatically moves it to its own space.  As far as Wine is concerned the 
> window resized — just like clicking the zoom button and manually assigning 
> the window to its own space.

Understood.

> I doubled the MaxTrackSize in user32/winpos.c, but it only  needs +15 for the 
> height.

This is a gross hack that's going to have to be reworked if this is ever going 
to get accepted. :)

If I'm understanding correctly, user32 is restricting the window to a size that 
keeps its caption (title bar) within the desktop.  Without this hack, the 
window resizes to be slightly smaller than would actually fill the screen.  Is 
that right?

Maybe we could add a new driver entry point to have user32 offer the driver the 
opportunity to override the default MINMAXINFO values.


>From cocoa_window.m.diff:

> +#import 


Was that actually needed?  cocoa_window.h already imports .

On a related note, to enable this to compile against the 10.6 SDK, we should 
have something like:

#if !defined(MAC_OS_X_VERSION_10_7) || MAC_OS_X_VERSION_MAX_ALLOWED < 
MAC_OS_X_VERSION_10_7
enum {
NSFullScreenWindowMask = 0,
NSWindowCollectionBehaviorFullScreenPrimary = 0,
NSWindowFullScreenButton = 7,
};
#endif

>  if (self.disabled)
>  style &= ~NSResizableWindowMask;
> -if (style != [self styleMask])
> +if (style != [self styleMask] && !([self styleMask] & 
> NSFullScreenWindowMask))
>  [self setStyleMask:style];

This is to avoid accidentally removing NSFullScreenWindowMask just because it's 
never in normalStyleMask, right?  I think a simpler solution is to just do:

style |= [self styleMask] & NSFullScreenWindowMask;

In fact, we should probably make a mask of the style mask bits that are ever 
valid in normalStyleMask and only tweak those.  That would be more future 
proof.  So, something like:

enum {
MACDRV_KNOWN_STYLES = NSTitledWindowMask | NSClosableWindowMask | 
NSMiniaturizableWindowMask | NSResizableWindowMask | NSUtilityWindowMask | 
NSBorderlessWindowMask
};
…
NSUInteger style = normalStyleMask | ([self styleMask] & 
~MACDRV_KNOWN_STYLES);


This:

> +if (style & NSResizableWindowMask)
> +[[self standardWindowButton:NSWindowZoomButton] 
> setEnabled:!self.disabled];


is to compensate for the fact that you might not call -setStyleMask:, above, 
right?  So it wouldn't be necessary after the change I suggested.  Furthermore, 
it isn't adequate.  You've disabled the zoom button but not the resize cursors 
all around the window border.

However, it does make me realize that we need to disable 
NSWindowFullScreenButton if the window is disabled.  Or is that what you meant 
to do here?  (We'd need to a protect against calling -standardWindowButton: 
with NSWindowFullScreenButton on OSes that don't support it.  It should suffice 
to test if [self respondsToSelector:@selector(toggleFullScreen:)].)


> -if (captured || fullscreen)
> +if (captured || (fullscreen && !(normalStyleMask & 
> NSResizableWindowMask)))

You're trying to distinguish between a window that was full-screened at the 
instigation of the Windows program versus one which has been full-screened by 
Cocoa, right?  Hmm.  I think we need a better technique to determine that.  I 
agree that it's not likely the Windows program would make its window 
full-screen while still allowing it to be resized but it feels wrong to assume 
that.

Also, I think we can entirely avoid setting the window level for windows that 
have been Cocoa full-screened.  Setting the window level is mostly about 
relationship to other windows, but a Cocoa full-screen window should be on a 
space by itself, shouldn't it?

> +else if ([self styleMask] & NSFullScreenWindowMask)
> +level = NSMainMenuWindowLevel + 2;

I'm not following why Cocoa full-screened windows should be at +2 vs. +1.

Also, fair warning: I've got some in-progress changes that will overhaul the 
management of window levels.  I don't think it will be too hard to adapt your 
patch, but methods will be moving or going away.


> +if (normalStyleMask & NSResizableWindowMask)
> +behavior |= NSWindowCollectionBehaviorFullScreenPrimary;

This is in -setMacDrvState: but, since normalStyleMask is changed in 
-setWindowFeature

Re: [1/3] winex11: Select for XRandR screen change events and implement an event handler.

[Ken Thomases]

On May 9, 2013, at 1:36 PM, Alexandre Julliard wrote:

> Handling external resizes
> should be done in the desktop process, and most likely involves the
> wineserver too. It certainly won't be an easy task.

Hmm.  Then that probably means I messed it up in the Mac driver, since what I 
did didn't seem so hard (and didn't involve the wineserver). ;)

-Ken

Re: [PATCH] kernel32: Fixed string comparision for non-canonical locale identifiers such as zh_CN VS zh-Hans on OS X.

[Ken Thomases]

On Apr 29, 2013, at 3:41 PM, Qian Hong wrote:

> Thanks Ken for many helps!

You're welcome and, regarding the patch: looks good to me.

-Ken

Re: opengl32: add a test for WGL_EXT_swap_control [try 2]

[Ken Thomases]

On Apr 29, 2013, at 10:42 AM, Roderick Colenbrander wrote:

> Updated version of the patch which marks the failing test as todo_wine. The
> final goal is to make the swap interval in the winex11 driver not global
> anymore (and store it with the drawable) and add some new functionality.
> <0001-opengl32-add-a-test-for-WGL_EXT_swap_control.patch>

For what it's worth, the Mac driver does it (apparently) wrong but in the other 
way.  In the Mac driver, the swap interval is currently part of the context 
state, so not part of the drawable.  But it's definitely not global.

-Ken

Re: Clang static analyzer results / wine-1.5.28-66-g6899279

[Ken Thomases]

On Apr 24, 2013, at 5:10 AM, Jacek Caban wrote:

> On 04/24/13 11:35, Frédéric Delanoy wrote:
> 
>> Although, one can sort/filter by file, or if it's really that
>> annoying, a quick script removing  elements containing "/tests"
>> should be easy enough.
> 
> Well... that's far from perfect, but could work.

Would it work to do two runs and have two different result sets?

I think that "make install" rather than "make" avoids building the tests 
without requiring --disable-tests.  (You can set DESTDIR to some temp directory 
in your home or /tmp.)  After that, a plain "make" will make just the tests.

-Ken

Re: [PATCH 2/2] gdi32: Clip font glyphs to fit within text metrics.

[Ken Thomases]

On Apr 23, 2013, at 2:45 AM, Sam Edwards wrote:

> On 04/22/2013 07:08 PM, Ken Thomases wrote:
>> I can't speak to how Windows would render it, but one such font is Zapfino. 
>> It's an "exuberant" calligraphic font with lots of flourishes, some of which 
>> have strokes extending into the line above or below. 
>> http://en.wikipedia.org/wiki/Zapfino https://www.google.com/images?q=zapfino 
>> -Ken 
> 
> I have never known about this font -- very stylistic indeed. :)
> 
> While there are plenty of strokes that go well above the cap height or below 
> the baseline, the Win Ascent and Win Descent values (which are defined as 
> yMax for all glyphs and -yMin for all glyphs, respectively) in the metric 
> tables are valid - none of the glyphs violate these limits, so the patch 
> doesn't affect this (correctly-formatted) font.
> 
> I loaded it up in Wine's Notepad, both with and without my patch, and took 
> screenshots of the output:
> http://cfsworks.com/files/images/screenshots/zapfino_wine_unpatched.png
> http://cfsworks.com/files/images/screenshots/zapfino_wine_patched.png

Hmm.  I assumed that ascent+descent+leading equals the distance between 
baselines.  In a Mac-native text editor, the strokes of one line of Zapfino can 
extend into the lines above or below, which is why I figured they exceeded the 
ascent and descent values. Notepad doesn't let the lines get that close, 
though.  So, something else must be going on.  I guess this isn't relevant to 
the question at issue with your patch.  Never mind. ;)

-Ken

Re: [PATCH 2/2] gdi32: Clip font glyphs to fit within text metrics.

[Ken Thomases]

On Apr 22, 2013, at 6:40 PM, Sam Edwards wrote:

> On 04/19/2013 10:32 AM, Max TenEyck Woodbury wrote:
>> As I understand it, some fonts deliberately have glyphs larger than
>> their metrics bounding boxes.  Clipping them is almost certainly not a
>> good idea.
> 
> Forgive my disbelief, but can you provide an example? It seems like Windows 
> has the same clipping behavior (see my test 
> http://source.winehq.org/patches/data/95792).
> 
> From my understanding, the intent of the ascent metric is that it indicates 
> the maximum ascender on any glyph in the font (and likewise for descent), so 
> the only real reason for the ascent/descent metrics to be wrong is if the 
> font designer made a mistake. (And some tools, like FontForge, will 
> automatically set the ascent/descent metrics correctly for you on export.)
> 
> I can't think of any reason why a font author would want to create a font 
> with an invalid ascent/descent metric.

I can't speak to how Windows would render it, but one such font is Zapfino.  
It's an "exuberant" calligraphic font with lots of flourishes, some of which 
have strokes extending into the line above or below.
http://en.wikipedia.org/wiki/Zapfino
https://www.google.com/images?q=zapfino

-Ken

Re: kernel32/path: Potential off-by-one error, help needed!

[Ken Thomases]

On Apr 22, 2013, at 1:17 PM, Kirill Smirnov wrote:

>While debugging bug #33307 (http://bugs.winehq.org/show_bug.cgi?id=33307) 
> I found a suspicious piece of code, looking like off-by-one error.
> 
>Unfortunately, I'm not familiar with this part of wine and I don't know 
> how to write corresponding test case.
> 
>If you familiar with this module, please, take a look at this 
> wanna-be-a-bug and the proposed patch.

I'm not familiar with the module, but on reviewing your patch and the local 
code it does appear that you have found a bug.  Your fix seems reasonable, 
although I think I'd change the memcpy() to use dest_name.Length + 
sizeof(WCHAR) as the length.  That way you don't have to duplicate the memcpy 
with two different lengths.

I don't think you have to worry about a test case.  This isn't a question about 
observable behavior of some Win32 API, it's just making Wine not write past the 
end of a buffer.

Cheers,
Ken

Re: winex11.drv: Transmit WM_DEADCHAR messages to applications

[Ken Thomases]

On Apr 16, 2013, at 3:43 AM, Simon Lipp wrote:

>> That's how it's supposed to work. 
> 
> But it doesn’t work that way under Windows. Shouldn’t Wine try to stick
> to Windows behavior as close as possible ?
> 
>> Wine should never generate WM_DEADCHAR message, it relies in the
>> driver to handle dead keys and generate final events.
> 
> I’m sorry but I fail to see any incompatibility between the two
> alternatives. Wine can generate WM_DEADCHAR messages AND let X/input
> method handle the real processing (and the generation of the final
> event) ; this patch never skip the XFilterEvent call, so it’s still up
> to your favorite IM to generate the final char from the keystrokes
> sequence.

For what it's worth, that agrees with my understanding, too.  The Mac driver 
supports dead keys in the manner that Simon suggests.  It generates WM_KEYDOWN 
for dead keys and returns -1 from ToUnicodeEx() for them, which causes 
TranslateMessage() to post WM_DEADCHAR.  When a subsequent key completes the 
sequence, ToUnicodeEx() returns the completed character(s) as indicated by the 
Mac keyboard layout.  It all seems to work fine.  (It also doesn't interfere 
with some local, in-progress work to support input methods.)

Nothing about posting WM_DEADCHAR takes control over the ultimate translation 
of keys into characters away from the driver or the windowing/input system with 
which it interfaces.

-Ken

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

[Ken Thomases]

On Apr 11, 2013, at 8:49 AM, Jacek Caban wrote:

> On 04/10/13 16:16, Ken Thomases wrote:
>> However, Apple's guidance on using weak linking says that you must 
>> explicitly compare against NULL.  They don't quite say that testing the 
>> symbol as a standalone boolean expression won't work, but they do say that 
>> using negation (the ! operator) isn't sufficient.
>> https://developer.apple.com/library/mac/documentation/developertools/conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW4
>> 
>> I'm not sure what the rationale is, but I think there's a peculiarity of 
>> what the compiler thinks it knows versus what the linker actually knows.  
>> Basically, the compiler figures there's a symbol that will be resolved (or 
>> cause a link error), so the check can be optimized away somehow.  I think.
> 
> Oh, such restriction seem to indicate a broken design of this weak
> linking... Following this is fine, through, but perhaps we should use
> nil to be sure that NULL override in winnt.h won't confuse the heuristic?

I expect that "nil" is only meaningful in Objective-C code.  I'd be surprised 
if there was anything special about NULL (either Apple's or Wine's), per se, 
but if you want an alternative that's insulated against any Wine peculiarities, 
I'd just use a literal "0" (zero).


>> Other than that, the patch looks good to me.
> 
> Thanks for the review.

Happy to help.

-Ken

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

[Ken Thomases]

Hi Jacek,

On Apr 10, 2013, at 4:24 AM, Jacek Caban wrote:

> On 3/28/13 8:31 PM, Ken Thomases wrote:
>> Mac OS X 10.8 introduced support for TLS 1.1 and 1.2.
> 
> Can someone with Mac OS X 10.8 test the attached patch for me, please. All I 
> need is to verify that it compiles and when running 
> dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are 
> listed as supported protocol.

Thanks for doing this.  I don't have a 10.8 build system handy, myself, or I 
would have done it or would test what you've done.

However, Apple's guidance on using weak linking says that you must explicitly 
compare against NULL.  They don't quite say that testing the symbol as a 
standalone boolean expression won't work, but they do say that using negation 
(the ! operator) isn't sufficient.
https://developer.apple.com/library/mac/documentation/developertools/conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW4

I'm not sure what the rationale is, but I think there's a peculiarity of what 
the compiler thinks it knows versus what the linker actually knows.  Basically, 
the compiler figures there's a symbol that will be resolved (or cause a link 
error), so the check can be optimized away somehow.  I think.

Other than that, the patch looks good to me.

-Ken

Re: winehtml5.drv: Added new HTML5 driver.

[Ken Thomases]

On Apr 1, 2013, at 6:41 AM, Jacek Caban wrote:

> There are still some missing features. Most notably, it lacks support
> for system tray baloons.

I know; that's, like, the hardest thing to do!

Great work, though!  Actually, since HTML is available everywhere, there's 
little reason for me to continue with the Mac driver.

-Ken

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

[Ken Thomases]

On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:

> --- a/dlls/secur32/schannel_macosx.c
> +++ b/dlls/secur32/schannel_macosx.c
> @@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef 
> transport, const void *buff,
>  return ret;
>  }
>  
> +DWORD schan_imp_enabled_protocols(void)
> +{
> +/* NOTE: No support for TLS 1.1 and TLS 1.2 */
> +return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | SP_PROT_TLS1_0_CLIENT;
> +}

Mac OS X 10.8 introduced support for TLS 1.1 and 1.2.  You can test at build 
time with:

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080
...
#else
...
#endif


If we want to support building on 10.8 for deployment to earlier versions, we'd 
do something like:

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080
SSLProtocol maxProtocol;
if (SSLGetProtocolVersionMax != NULL && 
SSLGetProtocolVersionMax(context, &maxProtocol) == noErr)
{
... compare maxProtocol against kTLSProtocol11 and 
kTLSProtocol12 ...
}
...
#else
...
#endif

The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd check 
if it was actually available before calling it.  Of course, the other 
complication is that that function requires a context parameter, but we can 
create one just for the query if we're interested in the framework capabilities 
(as opposed to what's been configured for a particular context).

-Ken

Re: winemac: Track drawn surface rect to reduce black flicker for new or resized windows.

[Ken Thomases]

On Mar 27, 2013, at 3:31 PM, Ken Thomases wrote:

> dlls/winemac.drv/macdrv.h  |   12 +++-
> dlls/winemac.drv/surface.c |   16 ++--
> dlls/winemac.drv/window.c  |   17 ++---
> 3 files changed, 27 insertions(+), 18 deletions(-)
> 
> <0001-winemac-Track-drawn-surface-rect-to-reduce-black-fli.patch>

Please don't commit this.  I've thought of a different approach.

-Ken

Re: winemac: Null-terminate an snprintf-ed key name buffer in case of short buffer.

[Ken Thomases]

On Mar 28, 2013, at 2:26 AM, Per Johansson wrote:

> On Wed, Mar 27, 2013 at 7:22 PM, Ken Thomases  wrote:
>> ---
> 
> This has probably been discussed before, but shouldn't snprintfW be
> safe against this kind of thing, the way POSIX snprintf is?

I actually checked the man page for snprintf() when I originally wrote that 
code, which is why I assumed snprintfW() would terminate it.  So, my vote is 
for "yes". :)

-Ken

Re: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution

[Ken Thomases]

On Mar 27, 2013, at 6:08 PM, Hugh McMaster wrote:

> Ken Thomases wrote:
> 
>>> This approach is untenable.  Kernel32 can't be made to depend on X11.  It 
>>> has to still work when X11 isn't available.  Also, X11 is just one possible 
>>> graphics/windowing system that Wine can use, so it's not always the right 
>>> authority to consult about screen size.
> 
> A fair point.  I hadn't considered that windowing systems such as XFree86 
> were still in use. I'm too used to using distributions running X11.

More to the point, Wine doesn't necessarily use any variant of the X Window 
System.  On Mac OS X, it can use the Mac driver.  Other drivers are being 
investigated.

>>> Finally, your patch series is broken in the sense that each patch in the 
>>> series must still be able to compile.  You introduce a dependency on 
>>> screensize.c in the first patch but don't actually provide that source file 
>>> until the third.
> 
> So patches are applied one by one and then tested?

Yes.

> I thought patches in a series were applied at the same time.

No.  A series is used to indicate that later patches can't be applied before 
the earlier ones.  That is, for example, patch 2 requires that patch 1 already 
was applied before it can be successfully applied.  "Successfully applied" 
means all of: the patch applies, Wine builds, and tests succeed (at least as 
well as before).

Regards,
Ken

Re: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution

[Ken Thomases]

On Mar 27, 2013, at 6:41 AM, Hugh McMaster wrote:

> This patch modifies the dlls/kernel32 Makefile.in to (1) compile a new source 
> file (screensize.c) (patch 3/3) and (2) add -lX11 to the ExtraDllFlags line.

This approach is untenable.  Kernel32 can't be made to depend on X11.  It has 
to still work when X11 isn't available.  Also, X11 is just one possible 
graphics/windowing system that Wine can use, so it's not always the right 
authority to consult about screen size.

Eric Pouech suggested in the other thread to calculate the console window size 
in wineconsole, at which point you can use user32 and SystemParametersInfo().

Finally, your patch series is broken in the sense that each patch in the series 
must still be able to compile.  You introduce a dependency on screensize.c in 
the first patch but don't actually provide that source file until the third.

-Ken

Re: explorer: Create desktop window hidden and only show it if it wins the race.

[Ken Thomases]

On Mar 26, 2013, at 3:20 AM, Dmitry Timoshkov wrote:

> Ken Thomases  wrote:
> 
>> +ShowWindow( hwnd, SW_SHOW );
>> SetWindowLongPtrW( hwnd, GWLP_WNDPROC, (LONG_PTR)desktop_wnd_proc );
>> SendMessageW( hwnd, WM_SETICON, ICON_BIG, (LPARAM)LoadIconW( 0, 
>> MAKEINTRESOURCEW(OIC_WINLOGO)));
>> if (name) set_desktop_window_title( hwnd, name );
> 
> You can postpone calling ShowWindow() after setting window text and icon,
> since they are drawn as part of a non-client area, and changing them
> forces another set of redraws.

Good idea.  Thanks.

-Ken

Mac driver screen flicker on launch (Was: Re: [1/3] Make mac driver the default on OS X)

[Ken Thomases]

On Mar 8, 2013, at 6:20 AM,  
 wrote:

> I had a very annoying experience: when I started winemac the first
> time, the whole screen turned black for a fraction of a second.  That
> was very unwelcome and reminded me of the infamous Intel-XOrg screen
> flickering issue that plagued many Linux distros a few years ago :-(

I have tracked this down.

Unless explorer.exe is launched explicitly, it is launched on demand.  Inside 
user32.GetDesktopWindow() there's a check if the desktop window exists.  If it 
doesn't, then explorer.exe is launched.

There is a race, of course, if multiple processes or threads call 
GetDesktopWindow() at roughly the same time before the desktop window has been 
created.  All of them start explorer.exe.

Explorer.exe creates a window that it intends to act as the desktop window.  It 
then checks if that window actually became the desktop window.  If it doesn't, 
it figures some other instance of explorer.exe won the race, destroys the 
window, and exits.  Only one of the explorer.exe instances survives and it owns 
the real desktop window.

The flicker is the prospective desktop window of the losers.  The windows are 
created visible, so they are briefly shown before being destroyed.

(The real desktop window is also created visible but it doesn't usually show 
unless virtual desktop is enabled under X11.  That's because both the Mac and 
X11 drivers have checks for the desktop window and treat it specially.  But 
only the real desktop window, from the winning explorer.exe, qualifies for that 
special treatment.  The windows from the losers are just normal windows.)

I think the solution is to have explorer.exe create its prospective desktop 
window hidden and only show it if it wins the race.  I'll submit a patch 
shortly.

-Ken

Re: Linker error when improving GetLargestConsoleWindowSize

[Ken Thomases]

On Mar 25, 2013, at 8:51 PM, Hugh McMaster wrote:

> I've written a function to determine the largest possible screen buffer that 
> wineconsole is able to display. This function replaces the hard-coded 
> constants listed in both instances of GetLargestConsoleWindowSize in 
> dlls/kernel32/console.c.
> 
> [snip]
> #include 
> #include 
> 
> [snip]
> 
> COORD GetLargestConsoleWindowSize_helper(HANDLE hConsoleOutput)
> {
>RECT workarea;
>COORD console_fontsize;
>COORD max_console;
> 
>SystemParametersInfoA(SPI_GETWORKAREA, 0, &workarea, 0);
>console_fontsize = GetConsoleFontSize(hConsoleOutput, 0);
> 
>max_console.X = (workarea.right / console_fontsize.X) - 6;
>max_console.Y = (workarea.bottom / console_fontsize.Y) - 6;
> 
>return max_console;
> }
> 
> While dlls/kernel32/console.c compiles, I'm having trouble with the final 
> compile-time linking.  It seems that SystemParametersInfoA is an undefined 
> reference, but windows.h and winuser.h are included in console.c.

This won't be able to work.  The linker error is telling you, effectively, that 
you're not importing user32.  However, kernel32 can't import user32.  Kernel32 
is at a lower level than user32.  And a console program especially can't rely 
on user32 being loaded and initialized to satisfy the request you're making.

Regards,
Ken

Re: winemac: Calculate surface rect correctly in macdrv_UpdateLayeredWindow().

[Ken Thomases]

On Mar 22, 2013, at 1:21 AM, Ken Thomases wrote:

> ---
> dlls/winemac.drv/window.c |   33 -
> 1 files changed, 16 insertions(+), 17 deletions(-)
> 
> <0001-winemac-Calculate-surface-rect-correctly-in-macdrv_U.patch>

Hmm.  On further consideration, I'm not sure this is right.  I think the intent 
was for the window surface of a ULW window to have a full surface even if it's 
partially off-screen.  So, perhaps macdrv_WindowPosChanging() needs to use the 
same calculation as macdrv_UpdateLayeredWindow() for that kind of window so the 
surface rect doesn't keep ping-ponging.

Also, the same issue exists in the X11 driver.

I'll have to discuss with Alexandre when he gets back.

-Ken

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

[Ken Thomases]

On Mar 21, 2013, at 3:41 PM, C.W. Betts wrote:

> On Mar 18, 2013, at 3:22 PM, Ken Thomases  wrote:
> 
>> On Mar 18, 2013, at 4:04 PM, Charles Davis wrote:
>> 
>>> At this point, though, I'm wondering if it wouldn't just be easier to have 
>>> Explorer draw the balloons itself à la Windows XP.
>> 
>> Probably.  I also considered using an NSPopover anchored to the status item. 
>>  Again, that's 10.7+ functionality.
> 
> We could implement it and do compile- and runtime checks to make sure that 
> the functions aren't called in unsupported OS X versions. However, this would 
> probably add fragmentation, and I doubt Alexandre would be happy with this.

I think he'd be OK with some variant of this.  He might not want two 
implementations, one using NSPopover on 10.7+ and another for 10.6.  I don't 
know how he'd feel about notification balloons only being implemented on 10.7+.

Although I'm not sure how important notification balloons are in the grand 
scheme of things, anyway.  I guess users will let us know if they miss them 
(unless somebody is inspired to take on that particular corner of unimplemented 
functionality beforehand).

Cheers,
Ken

Re: winemac: Implement GetDeviceGammaRamp() and SetDeviceGammaRamp().

[Ken Thomases]

On Mar 20, 2013, at 1:33 PM, André Hentschel wrote:

> Am 18.03.2013 04:41, schrieb Ken Thomases:
>> ---
>> dlls/winemac.drv/display.c |  131 
>> 
>> dlls/winemac.drv/gdi.c |4 +-
>> dlls/winemac.drv/macdrv.h  |2 +
>> 3 files changed, 135 insertions(+), 2 deletions(-)
> 
> Hi,
> wrt macdrv_SetDeviceGammaRamp, the X11 driver has some validation in 
> xvidmode.c->ComputeGammaFromRamp() because windows doesn't allow every ramp.
> It might happen that apps try bad ramps and the screen simply gets dark 
> without validation.

Ah, interesting.  I missed that.

> I'll see if i can copy/move the validation to gdi32 where the driver function 
> normally get's called so that the validation is driver independent.

Thanks!

-Ken

Re: Mac driver status: ready for contributions

[Ken Thomases]

On Mar 19, 2013, at 1:31 AM, Charles Davis wrote:

> I have one more of my own to add:
> 
> * Fix crash whenever an app makes a GLU call.

Ah, yes, I forgot that one.  Thanks for reminding me.


> I suspect this is because Mesa's GLU calls Mesa's libGL--which crashes 
> horribly because there's no Mesa/GLX context current.

Yes, basically.


> I see two ways to fix that. One is to make GLU dispatch go through the driver.

This is what I did for CrossOver 12.  The problem is that it introduced a 
requirement that a WGL context be current on the thread to find the driver 
functions.  (Technically, the GLU spec requires a current GL context at the 
time of any GLU calls, but glu32 doesn't require that and, empirically, it 
seems that Mesa's library doesn't either.  Because some stuff that used to work 
was broken by CrossOver 12.)

> The other is to actually implement glu32 (i.e. not as a forwarder to the 
> system GLU). I'm not sure how AJ will feel about the first one--and I 
> probably won't know until next week.

Not thrilled, although willing to entertain it if it can be done in a 
relatively elegant and robust way.  None of my proposals rose to those 
standards. ;)

> The second one... writing a GLU is a lot of work. We could use SGI's (as Mesa 
> does), but SGI GLU is largely written in C++--and I *know* AJ won't like that.

Implementing glu32 on top of opengl32 was Alexandre's approach.  He was looking 
for an open-source GLU implementation to use.  He could find C implementations 
for most parts of GLU except the NURBS stuff.

We didn't really get any further than that.

-Ken

Re: [10/10] winemenubuilder: Put appbundle icon inside the bundle.

[Ken Thomases]

On Mar 17, 2013, at 12:18 PM, Per Johansson wrote:

> +CFStringRef iconstr = CFStringCreateWithCString(NULL, icon, 
> CFStringGetSystemEncoding());

Another place to use CFStringCreateWithFileSystemRepresentation().

-Ken

Re: [7/10] winemenubuilder: Create a basic Info.plist.

[Ken Thomases]

On Mar 17, 2013, at 12:18 PM, Per Johansson wrote:

> +namestr = CFStringCreateWithCString(NULL, link_name, 
> CFStringGetSystemEncoding());

You should use CFStringCreateWithFileSystemRepresentation() to create CFStrings 
from POSIX path strings.


> +pathstr = CFStringCreateWithCString(NULL, path, 
> CFStringGetSystemEncoding());

Same thing here.


> +data = CFPropertyListCreateData(NULL, propertyList, format, 0, &err);

Since you don't use "err", just pass NULL.

-Ken

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

[Ken Thomases]

On Mar 18, 2013, at 4:04 PM, Charles Davis wrote:

> On Mar 18, 2013, at 2:23 PM, Ken Thomases wrote:
> 
>> On Mar 18, 2013, at 2:15 PM, Ken Thomases wrote:
>> 
>>> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote:
>>> 
>>>> On Mar 17, 2013, at 9:40 PM, Ken Thomases  wrote:
>>>> 
>>>>> Doesn't support right-clicks, mouse moves, or notification balloons.
>>>> Notification balloons can probably be done using either Notification 
>>>> Center or Growl
>>> 
>>> Yeah, I was thinking that the notification center is the right way to go.
>> 
>> Josh reminds me that only code-signed apps can use the Notification Center, 
>> so that may not be very useful. *sigh*
> So what? [...]
> 
> Though I wonder if it's a good idea to ask users to create a self-signed 
> certificate just so Wine can use the Notification Center...

You answered your own question. ;)


> At this point, though, I'm wondering if it wouldn't just be easier to have 
> Explorer draw the balloons itself à la Windows XP.

Probably.  I also considered using an NSPopover anchored to the status item.  
Again, that's 10.7+ functionality.

-Ken

Mac driver status: ready for contributions

[Ken Thomases]

Hi,

With my latest set of patches, the Mac driver in Wine is up to speed with 
what's in CrossOver 12 (plus some extra goodies).

Of course, that's not to say that the Mac driver is done, but if you were 
holding off getting involved because you figured big chunks of already-done 
work were pending, that's no longer the case.  From here on out, all work on 
the Mac driver will be new coding, so it's ready for anybody to contribute.

Testing is one excellent way of contributing.  I encourage any Mac Wine users 
to enable the Mac driver, test your favorite programs, and file bugs for any 
new problems (and there _will_ be bugs to find).  Please try to distinguish 
between general Wine bugs which also happen with the X11 driver vs. bugs which 
only happen with the Mac driver.

Currently, you have to set a registry key to enable the Mac driver:

[HKEY_CURRENT_USER\Software\Wine\Drivers]
"Graphics"="mac,x11"

Swap the order to "x11,mac" – or just remove the "Graphics" value altogether – 
to revert to using the X11 driver.


Aside from bugs, the Mac driver is pretty close to complete.  Here's a list of 
stuff that I know is incomplete.  Not all of these things are hard.  In fact, I 
expect some will be quite easy and the only reason I didn't get to them was to 
get broader coverage of general areas of functionality by CrossOver 12's 
release.

* The Quit menu item should do a graceful shutdown of the Windows process 
rather than abruptly terminating it.  Same for requests to quit from the system 
such as during logout, restart, or shutdown.

* A registry setting to govern whether full-screen at full-resolution should 
capture the display (which will disable active screen corners but also 
application switching and UI overlays).  Full-screen at other resolutions 
already captures the displays.

* A registry setting to govern whether the Option/Alt key sends an Alt 
keystroke instead of accessing additional characters from the Mac keyboard 
layout.

* Support for maximizing windows.

* Reduce or eliminate flashes of black before windows (re)draw.

* Support for Mac input methods (Asian languages, press-and-hold, keyboard and 
character viewers).

* Rework the Graphics tab of winecfg.  Most of it doesn't apply to the Mac 
driver.  As Mac-driver-specific registry settings are implemented, they may 
need to be revealed in winecfg.

* Maybe implement a Cocoa Preferences dialog for the Mac driver's settings as 
an alternative to putting them in winecfg.

* Activating a Wine GUI process all of whose windows are minimized should maybe 
unminimize one.

* Add some additional standard items to the Mac menus, such as Hide Wine, Hide 
Others, and Show All.

* Support for metafile clipboard formats.

* Support for mouse capture.

* Improved system tray functionality like detecting mouse moves and 
right-clicks and presenting notification balloons.

* Support localization of the few UI strings in the Mac driver (basically the 
menu item titles).

(Charles Betts has already submitted work to support disabling and reenabling 
the screen saver.  As I understand it, Alexandre has some unspecified ideas 
about how he wants to change the driver interface for the SystemParametersInfo 
entry point.)

Cheers,
Ken

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

[Ken Thomases]

On Mar 18, 2013, at 3:15 PM, Ken Thomases wrote:

> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote:
> 
>> On Mar 17, 2013, at 9:40 PM, Ken Thomases  wrote:
>> 
>>> Doesn't support right-clicks, mouse moves, or notification balloons.
>> Notification balloons can probably be done using either Notification Center 
>> or Growl
> 
> Yeah, I was thinking that the notification center is the right way to go.

Josh reminds me that only code-signed apps can use the Notification Center, so 
that may not be very useful. *sigh*

-Ken

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

[Ken Thomases]

On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote:

> On Mar 17, 2013, at 9:40 PM, Ken Thomases  wrote:
> 
>> Doesn't support right-clicks, mouse moves, or notification balloons.
> Notification balloons can probably be done using either Notification Center 
> or Growl

Yeah, I was thinking that the notification center is the right way to go.  Of 
course, we need to code support for it in a way that still builds and runs on 
10.6.

I'm not sure if we want to use Growl given that it's not part of the OS, but I 
guess I don't feel very strongly one way or another.

-Ken

Re: winemac: fix compilation on Lion and later

[Ken Thomases]

On Mar 7, 2013, at 4:15 PM, C.W. Betts wrote:

> This patch defines __gltypes_h_ so that the Mac header doesn't get included.

Thanks!

-Ken

Re: [1/3] Make mac driver the default on OS X

[Ken Thomases]

On Mar 7, 2013, at 5:12 PM, Charles Davis wrote:

> In the case that the window doesn't have its own grow box, though, it can be 
> hard to tell that you actually still can resize the window by dragging the 
> corner. (In notepad's case, it's weird, because I can click and drag the Down 
> button on the scroll bar to resize the window. It happens to be about where 
> the native grow box would have been drawn if it weren't turned off.) On 
> Windows, resizable windows have at least a thicker frame (THICKFRAME style), 
> and they usually also have the CLIENTEDGE and WINDOWEDGE extended styles 
> (giving the appearance of a frame that you can grab and drag to resize the 
> window). The Quartz window server precludes that with its pixel-thin border 
> and insistence that you grab the lower-right corner--at least, prior to Lion. 
> That's why I suggested changing the cursor to SIZENWSE at the lower-right 
> corner. I should really write a patch that does that :).

Yeah, maybe changing the cursor is right, although, again, the Windows app 
should be responsible for the cursor within its borders.

The driver is responsible for how much of the window border shows or doesn't, 
though.  See get_mac_rect_offset().  It may make sense to expose some of them 
if they're thick and resizable.  Might look bad, though.

>>> Why aren't we using Option as Alt? Nearly every Mac keyboard labels it as 
>>> "Alt" these days. Is typing characters like 'ü' and 'ø' and '∆' really that 
>>> important? :)
>> 
>> Given the smiley, I can't tell if you're joking or not,
> Yeah, I'm joking. I realized just how silly taking that away would be as I 
> was typing.
>> but, yes, accessing additional characters from the keyboard layout is 
>> important.  CodeWeavers' experience with support requests actually strongly 
>> influenced my decision to reserve Option for that.  I'm open to introducing 
>> a registry setting to allow that to be changed.
> 
> If we introduce a registry setting, should we also add a Control Panel 
> (something akin to Windows' Keyboard applet) or Preference Pane or something 
> to change it?

I have considered a Cocoa preferences dialog accessible from the Mac menu bar.  
It would show and modify any future Mac-driver-specific registry settings, 
similar to winecfg's Graphics tab for the X11 driver.  There would be a radio 
button to change whether it's working with the settings for just the current 
executable or all executables.

Some programs, though, will run full-screen, making the preferences dialog 
inaccessible.  So, winecfg would probably need to let users edit Mac driver 
settings, too.  In that case, a preferences dialog is duplicated effort that 
maybe shouldn't be done.


> And if we do, we may wish to include a Character Map (charmap.exe) utility so 
> users can insert additional characters anyway.

No.  The right solution is to integrate with Mac input methods, including the 
Keyboard Viewer and Character Viewer.  I've been working on that.  (CrossOver 
has an implementation which works with Asian-language input methods, but not 
with the viewers or palettes.  It also doesn't support the press-and-hold 
mechanism introduced with Lion.  I hope to support all of these.)


> While we're on the subject of bugs: another annoying problem is that, when a 
> window is first created, the view appears black, and then the contents are 
> filled in. This happens even with minor windows like menus. (It happens with 
> the X11 driver too, just less often.)

Yeah.  Also, with the X11 driver, the windows are white before they're drawn 
rather than black, making the problem less noticeable to the eye, I think.

The Mac driver is using a "pull" model to get the drawn bits from the window 
surface.  The problem is that Cocoa asks us to draw as soon as the window is 
ordered on-screen, but the surface is still blank at that point.  A short while 
later, the surface gets drawn (at least partially) and user32 tells us to 
flush.  We tell Cocoa that the view needs to be redrawn and Cocoa asks us to 
draw, at which point we pull the drawn bits.

My best thought for how to fix this is to simply defer ordering the window onto 
the screen until the surface is flushed.

A similar problem arises when windows are resized, and the solution may be the 
same: defer the actual resize until the surface has been flushed.

-Ken

Re: [1/3] Make mac driver the default on OS X

[Ken Thomases]

On Mar 7, 2013, at 12:03 PM, Charles Davis wrote:

> On Mar 6, 2013, at 7:56 AM, Ken Thomases wrote:
> 
>> On Mar 6, 2013, at 8:24 AM,  
>>  wrote:
>> 
>>> Bug report:
>>> Toying around with built-in notepad, clock, winhlp32, I noticed that
>>> notepad and winhlp32 are not resizable, whereas the system
>>> preferences "control" is.  By comparison, with the x11 driver,
>>> notepad's window is resizable even though it has no triangle
>>> widget at the bottom left.
>> 
>> Works here™.  I can resize notepad and winhlp32 just fine.
> The problem is that the ability to resize windows by dragging their borders 
> with the mouse was added in Lion. I'll bet Jörg is running Snow Leopard, 
> where you can only resize windows by dragging the size box in the lower right 
> (for me, anyway) corner.

I'm running Snow Leopard, myself.

> Except that many windows don't even have size boxes, even though you can 
> still resize them by clicking and dragging their lower-right corner.
> 
> If a window doesn't have a size box already (usually, a status bar with the 
> SBARS_SIZEGRIP style), we should add a transparent one ourselves on SL, 
> similar to what Xplugin does. Or, we should change the cursor into 
> IDC_SIZENWSE when the pointer hovers over the corner.

Alexandre specifically suggested that I suppress the grow box in the Mac driver 
since drawing resize widgets should be up to the Windows app, so that's what I 
did.  It's the call to [window setShowsResizeIndicator:NO] in cocoa_window.m.

Also, Jörg already was aware that windows may be resized when they don't show 
grow box, since he noted that there isn't one with the X11 driver.  So what he 
reported is something else.


>>> Also, the virtual desktop seems gone or not yet implemented.  This is a key
>>> requirement for enough apps that do not handle today's huge resolutions.
>> 
>> There is no mechanism for one Mac process to draw into the windows of 
>> another.
> But there is--an indirect one, anyway. You can have everyone else draw into 
> an IOSurface, and then share the IOSurface with Explorer (which I assume is 
> responsible for managing the desktop window). You can even use an IOSurface 
> as a GL texture (and, by extension, render with OpenGL into the IOSurface). 
> This is how Chrome (and probably Safari too, since it uses the same 
> underlying engine) implements sandbox'd rendering: the renderer draws the 
> page into an IOSurface, then the browser draws the IOSurface into the window. 
> And yes, this will work on Snow Leopard.

I'm aware of IOSurface.  It's one of the shared-memory mechanisms I mentioned 
as being in consideration.  As always, patches welcome!  ;)


>>> Another annoyance was that there's no HIDE function or short cut and some
>>> dialogues do not provide the orange button (middle one) to iconify the 
>>> window.
>>> As a result, those pesky windows clutter your screen.
>>> That alone is enough reason to me a reason not to use that driver. It is not
>>> acceptable that one cannot get rid of a set of windows!
>>> Example: wine control, then navigate to the root CA certificates.
>> 
>> More items can be added to the menus easily enough.  However, I'm loathe to 
>> assign the usual keyboard shortcuts.  The problem is that the Command key is 
>> used to generate Alt keystrokes and it's relatively likely that a Windows 
>> program will want to receive Alt-H for its menus.
> Why aren't we using Option as Alt? Nearly every Mac keyboard labels it as 
> "Alt" these days. Is typing characters like 'ü' and 'ø' and '∆' really that 
> important? :)

Given the smiley, I can't tell if you're joking or not, but, yes, accessing 
additional characters from the keyboard layout is important.  CodeWeavers' 
experience with support requests actually strongly influenced my decision to 
reserve Option for that.  I'm open to introducing a registry setting to allow 
that to be changed.

And, yes, it sucks that the key is labelled Alt but can't be used as Alt. :(

-Ken

Re: [PATCH 1/7] winemac: Implement OpenGL support.

[Ken Thomases]

On Mar 7, 2013, at 9:39 AM, C.W. Betts wrote:

> It looks like if GL_ARB_shader_objects is defined, then the OS X GLhandleARB 
> isn't typedef'd.

I think a better approach is to define __gltypes_h_ just to no-op that whole 
header.  Does that fix compilation for you?  If so, please submit it as a patch.

Thanks,
Ken

Re: [PATCH 1/7] winemac: Implement OpenGL support.

[Ken Thomases]

On Mar 6, 2013, at 1:58 PM, C.W. Betts wrote:

> It seems like there's conflicting types for GLhandleARB, one defined by Wine, 
> the other by OS X, on Mountain Lion. I assume that the #define __gl_h_ is a 
> try to work around the issue, but OS X's gltypes.h still gets included.

Ugh.  Thanks for letting me know.  I'll have to investigate.  I'm not well set 
up to build against later SDKs at the moment, but I'll see what I can do.  Of 
course, if you can figure out a solution, that would be great.

-Ken

Re: [1/3] Make mac driver the default on OS X

[Ken Thomases]

On Mar 6, 2013, at 8:24 AM,  
 wrote:

> Bug report:
> Toying around with built-in notepad, clock, winhlp32, I noticed that
> notepad and winhlp32 are not resizable, whereas the system
> preferences "control" is.  By comparison, with the x11 driver,
> notepad's window is resizable even though it has no triangle
> widget at the bottom left.

Works here™.  I can resize notepad and winhlp32 just fine.


> Also, the virtual desktop seems gone or not yet implemented.  This is a key
> requirement for enough apps that do not handle today's huge resolutions.

There is no mechanism for one Mac process to draw into the windows of another.  
That's key to how the virtual desktop is implemented.  So, it's not really 
possible as things stand.  Alexandre and I have considered some sort of 
shared-memory mechanism, but it's barely into the conceptualization stage, let 
alone design or implementation.

Alternatively, we'd have to recreate X11 – a protocol for shuttling drawing 
operation requests from one process to another – to fix this.

I'm not sure it's worth it.  In my experience, the virtual desktop has more 
often been used to work around X window manager limitations.  The hope is that 
we have greater control with the Mac driver.  If all else fails, the X11 driver 
is still available.


> Another annoyance was that there's no HIDE function or short cut and some
> dialogues do not provide the orange button (middle one) to iconify the window.
> As a result, those pesky windows clutter your screen.
> That alone is enough reason to me a reason not to use that driver. It is not
> acceptable that one cannot get rid of a set of windows!
> Example: wine control, then navigate to the root CA certificates.

More items can be added to the menus easily enough.  However, I'm loathe to 
assign the usual keyboard shortcuts.  The problem is that the Command key is 
used to generate Alt keystrokes and it's relatively likely that a Windows 
program will want to receive Alt-H for its menus.  Or, perhaps, a game uses Alt 
for a player action (e.g. crouch) and H for another action and the user would 
end up pressing them simultaneously without even considering them a key 
combination.

My principle has been that only keyboard shortcuts that combine Command and 
Option should be used, since that's unlikely to be pressed by accident.  In the 
case of the Hide menu item, though, Command-Option-H would ordinarily mean Hide 
Others, just the opposite of Hide.

In any case, for the meantime, you can hide a Wine app from its Dock menu.


> The menu bar sometimes was refreshed badly, causing some texts therein
> to be displayed twice, shifted by a few pixels, or with varying fonts.

I'm not familiar with that, but it's not likely to be the fault of the Mac 
driver.  The Mac driver is not really a "graphics" driver.  It doesn't draw 
anything.  It uses Wine's relatively new client-side graphics implementation.  
User32 arranges for the DIB engine to do all of the drawing and then just 
deliver bitmaps to the Mac driver for it to blit to the window.

Are you sure you aren't see the same thing with the X11 driver?

-Ken

Re: [3/3] Make mac driver the default on OS X

[Ken Thomases]

On Mar 5, 2013, at 6:32 PM, Josh DuBois wrote:

> diff --git a/dlls/winemac.drv/macdrv_main.c b/dlls/winemac.drv/macdrv_main.c
> index cd1cc0c..1f16694 100644
> --- a/dlls/winemac.drv/macdrv_main.c
> +++ b/dlls/winemac.drv/macdrv_main.c
> @@ -85,7 +85,7 @@ static BOOL process_attach(void)
>  macdrv_err_on = ERR_ON(macdrv);
>  if (macdrv_start_cocoa_app(GetTickCount64()))
>  {
> -ERR("Failed to start Cocoa app main loop\n");
> +TRACE("Failed to start Cocoa app main loop\n");
>  return FALSE;
>  }
>  

I would think that this should still be at least a WARN, although I'm not sure 
why it's being demoted from ERR in the first place.  (In fact, I should 
probably have added a WARN in my patch for refusing to run in non-GUI sessions.)

-Ken

Re: [1/3] Make mac driver the default on OS X

[Ken Thomases]

On Mar 5, 2013, at 6:32 PM, Josh DuBois wrote:

> ---
> dlls/gdi32/driver.c   |  102 +++--
> dlls/gdi32/gdi32.spec |1 +
> dlls/gdi32/gdi_private.h  |2 +
> dlls/user32/driver.c  |   27 ++--
> include/wine/gdi_driver.h |   11 +
> 5 files changed, 125 insertions(+), 18 deletions(-)
> 
> <0001-Track-and-report-multiple-errors-loading-display-drive.txt>

As I believe you're aware, I think that the Mac driver should not be made the 
default until OpenGL and clipboard support are in.  So, maybe this was just 
submitted in preparation for that time, but there should probably have been a 
note to that effect.

Also, this patch seems exceedingly complex for a relatively simple problem.  
Just because it has historically been user32 that reported failure to load the 
driver doesn't mean it must remain so, at least to my way of thinking.  Why not 
just print out the diagnostics directly in gdi32 as each attempt to load a 
driver fails?  In user32, there can be a secondary, driver-agnostic message 
about failure to create a window.


> +case ERROR_DLL_INIT_FAILED:
> +load_err->err_msg = "Make sure you have permission to create \na 
> window or check for errors with Console.app. \n(The Mac driver cannot do 
> remote display: try X11 if you need that.)";
> +break;


"Make sure you have permission to create a window" doesn't mean anything to me. 
 Checking Console.app isn't very useful either.  If the user sees this message, 
then they're already looking at wherever the process's output is going.  
Running Wine from a shell won't write anything to the console logs.  As of 
Mountain Lion, running it from a GUI app won't either; Apple changed things so 
that stdout and stderr of GUI apps go to /dev/null rather than console logs.  
(Only things like NSLog() or asl_log() go to the console log.)

Probably it's just best to say that the Mac driver is running in a non-GUI 
session such as a remote login.

-Ken

Re: Making wine default to the Mac Driver on OS X

[Ken Thomases]

My feeling (like Josh's and Per's) is that the Mac driver should be what you 
get by default, in most cases, without taking any special steps.

If the concern is what you get when you're ssh'd into a remote system, then 
it's probably possible for the Mac driver to detect when it doesn't have GUI 
console access and fail.  Then, Wine could fall back to X11.

We can't rely on DISPLAY being unset.  It's set by default.  That's true on 
Snow Leopard and Lion.  I believe it's also true on stock Mountain Lion 
(whether or not XQuartz has been installed).

(The question of when to make this switch so that the Mac driver is the default 
is separate.  OpenGL is my next major patch set.  I hoped to submit it 
yesterday, but got tripped up by some changes in Wine since the CrossOver 12 
base.)

-Ken

Re: [PATCH] ntdll: Support a protection scheme which reset the GS selector - v3

[Ken Thomases]

On Feb 17, 2013, at 4:18 AM, Alessandro Pignotti wrote:

> Third attempt
> 
> 
> 
> Hi everyone,
> 
> I've found two different games:
> 
> -) Of orcs and men
> -) The testament of sherlock holmes
> 
> which are using a protection scheme which reset the GS segment selector,
> possibly to confuse virtual machines. Since on linux the GS selector is
> used internally by glibc the game would crash as soon as a native glibc
> method is called by wine. The proposed patch tries to workaround this
> problem by restoring the GS to a known working value in wine's SEGFAULS
> handler and try the instruction again.
> 
> Regards,
> Alessandro Pignotti
> 
> <0001-ntdll-Support-a-protection-scheme-which-reset-the-GS-v3.patch>

Didn't Alexandre already fix this with 
?

-Ken

Re: Keynote made me remember Mac Icons

[Ken Thomases]

Hi,

On Feb 3, 2013, at 3:56 AM, Austin English wrote:

> On Sun, Feb 3, 2013 at 9:45 AM, Jeremiah Flerchinger
>  wrote:
>> The keynote made me realize I have some Mac icons for Wine. Would anyone be
>> interested in them? It's definitely more functional than just using the
>> default terminal icon. I also had a winecfg icon somewhere but I can't
>> remember where that one is right now.
>> 
>> Let me know and tell me where to send them.
> 
> Ken Thomases (cc'ed) is the person on winemac.drv, though wine-devel
> or attaching them to bugzilla would likely be best to preserve a copy
> of it.

I may be missing some context.  What icons is this referring to?  Do you mean 
just the various icons for Wine and its programs (e.g. Notepad) but in ICNS 
format?

I'm not sure yet how useful that will be.

Although I haven't gotten this far in my Mac driver patch submissions, the Mac 
driver will eventually attempt to extract the icon for the current .exe and use 
that for the application icon of the running process.  Not all executables have 
an icon, though, so in some cases it will still default to the terminal-looking 
icon.  It probably would be better to use Wine's wine-glass icon in that case.

However, there's a question of how to deploy the icon.  I'm not sure a .icns 
file is useful.  I don't know where such a .icns file would be kept and how it 
would be located relative to the Wine installation (or build directory, if 
running from there).  So, it may just be easiest to embed a Windows ICO 
resource into the Mac driver .so file and extract it at runtime just like the 
code will extract the icon from the .exe file.

So, thanks for the offer and I'll keep it in mind, but at this point I'm not 
sure what I would do with Mac icons for Wine.

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 6

[Ken Thomases]

On Jan 22, 2013, at 5:33 PM, C.W. Betts wrote:

> Yet another fix.

Looks good to me.  Thanks for doing this and for putting up with me.

Cheers,
Ken

Re: macdrv: implement getting and setting the screen saver state. if(Style)

[Ken Thomases]

On Jan 22, 2013, at 6:38 AM,  
 wrote:

> Ken Thomases wrote:
>>> +if(success != kIOReturnSuccess)
>> Another style nitpick: please put a space between "if" and the condition.
>> That applies to the "if(count2)" above, too.
> 
> I find this over the top.

Well, I phrased it as a style nitpick (a.k.a. a matter of taste and a minor 
issue).  I also said "please".  I don't think it was too over the top.

> Where is the rule?

> … The only rule I see is: "Look at the code surrounding the patch" and
> its corollary: "if you create a new module, you're the first and define
> the style others will have to follow."

Well, there's also the corollary "be self-consistent within a single patch" 
which applied in this case.  Those guidelines (I wouldn't call them "rules") 
seem sane and fairly self-evident.  In any case, they are already documented 
here:
http://wiki.winehq.org/SubmittingPatches#head-22ead28485239d5028d63061c7d38f0fa88230d8

I have no power to enforce anything.  Alexandre may or may not consider my 
patch reviews, or those of anybody else, while deciding how to treat the 
patches in his incoming queue.  The reviews are simply a joint effort of all 
Wine developers to help keep quality up and take some of the burden off of 
Alexandre.

If the code submitter chooses to ignore my suggestions, they can.  I probably 
won't keep harping on the same minor style issue for future versions.

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 5

[Ken Thomases]

On Jan 22, 2013, at 12:22 PM, C.W. Betts wrote:

> This version should fix the problems you mentioned

Sorry about this – I should have thought about this last time – but I think you 
shouldn't fall through in the SPI_SETSCREENSAVEACTIVE case.  It's asking for 
trouble in the future, if anybody adds another case to the switch statement.

I think it would be better to just explicitly return FALSE and put in a comment 
explaining why (like you suggested for the X11 driver).

Thanks for your persistence!

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 4

[Ken Thomases]

Hi,

On Jan 21, 2013, at 6:21 PM, C.W. Betts wrote:

> This version falls through in SPI_SETSCREENSAVEACTIVE.

Sadly, you used C++ style for the fall-through comment. :-/

Also, you have some trailing whitespace and the patch no longer applies cleanly 
after today's commits of mine.  It should be easy to fix that though.  There 
aren't any serious conflicts.

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 3

[Ken Thomases]

On Jan 21, 2013, at 11:50 AM, C.W. Betts wrote:

> This version gets rid of redundant comments.

According to Alexandre's comment on your X11 driver patch, you should be 
returning FALSE from the "SET" case so that user32 will update the registry.  
Otherwise, looking good.

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 2

[Ken Thomases]

On Jan 21, 2013, at 9:31 AM, C.W. Betts wrote:

> On Jan 20, 2013, at 11:06 PM, Ken Thomases  wrote:
> 
>> I'm not sure it's correct that these are two different names for roughly the 
>> same thing.  I think the two assertion types do slightly different things.  
>> "NoDisplaySleep" seeks to prevent display sleep for any (non-forced) reason. 
>>  "PreventUserIdleDisplaySleep" only seeks to prevent display sleep that is 
>> due to user idleness.
> From the header for kIOPMAssertionTypeNoDisplaySleep: Deprecated in 10.7; 
> Please use assertion type kIOPMAssertionTypePreventUserIdleDisplaySleep. From 
> Apple Docs, kIOPMAssertionTypePreventUserIdleDisplaySleep is only available 
> on 10.7 or later.

I can't be certain, because Apple hasn't documented this API very well, but I 
believe that NoDisplaySleep is deprecated because Apple doesn't like its 
semantics.  It's not because it's just an old name for 
PreventUserIdleDisplaySleep; it's because PreventUserIdleDisplaySleep has 
better semantics.

> I don't think kIOPMAssertionTypePreventUserIdleDisplaySleep will do anything 
> on 10.6

Right.

I wasn't suggesting that you use PreventUserIdleDisplaySleep on 10.6.  The code 
itself is fine.  I'm just saying that your comments were misleading because 
they suggest there are two different names for the same thing.

-Ken

Re: macdrv: implement getting and setting the screen saver state, version 2

[Ken Thomases]

Hi,

On Jan 20, 2013, at 2:00 PM, C.W. Betts wrote:

> This version implements changes and advice from Ken Thomases, including using 
> kIOPMAssertionTypePreventUserIdleDisplaySleep on Lion and later. Also, some 
> comments were added.

> +//Get pre-Lion no display sleep counts

C++-style comments ("//") are not allowed in Wine C code.  They aren't portable.


> +CFNumberRef count = 
> CFDictionaryGetValue(assertionStates, kIOPMAssertionTypeNoDisplaySleep);
> +CFNumberRef count2 = NULL;
> +long longCount = 0;
> +long longCount2 = 0;
> +CFNumberGetValue(count, kCFNumberLongType, &longCount);
> +//If available, get Lion and later no display sleep 
> counts
> +count2 = CFDictionaryGetValue(assertionStates, 
> kIOPMAssertionTypePreventUserIdleDisplaySleep);
> +if (count2)
> +CFNumberGetValue(count2, kCFNumberLongType, 
> &longCount2);

The issue discussed for the previous patch is still present.  For example, the 
following code would be internally consistent and also safe:

CFNumberRef count = CFDictionaryGetValue(assertionStates, 
kIOPMAssertionTypeNoDisplaySleep);
CFNumberRef count2 = CFDictionaryGetValue(assertionStates, 
kIOPMAssertionTypePreventUserIdleDisplaySleep);
long longCount = 0;
long longCount2 = 0;
if (count)
CFNumberGetValue(count, kCFNumberLongType, &longCount);
if (count2)
CFNumberGetValue(count2, kCFNumberLongType, 
&longCount2);

(Note that this reorganization also eliminates a dead store.  You were 
initializing count2 and then unconditionally assigning it, making the 
initialization redundant.  Such unnecessary initializations also defeat 
potentially valuable compiler warnings.)


> +if (longCount || longCount2 )

There's an extraneous space before the closing parenthesis.


> +//Release power assertion
> +IOReturn success = IOPMAssertionRelease(powerAssertion);


The comment is useless.  It tells us nothing that the code doesn't.  Please 
don't do that.  In fact, most of the comments you added are like this.


> +//Are we running Lion or later?
> +if (kCFCoreFoundationVersionNumber >= 
> kCFCoreFoundationVersionNumber10_7)
> + //If so, use Lion's name
> +assertName = 
> kIOPMAssertionTypePreventUserIdleDisplaySleep;
> +else
> + //If not, use old name
> + assertName = kIOPMAssertionTypeNoDisplaySleep;

I'm not sure it's correct that these are two different names for roughly the 
same thing.  I think the two assertion types do slightly different things.  
"NoDisplaySleep" seeks to prevent display sleep for any (non-forced) reason.  
"PreventUserIdleDisplaySleep" only seeks to prevent display sleep that is due 
to user idleness.

As I suggested previously, I do want us to use PreventUserIdleDisplaySleep when 
it's available, so that's good.  (Thank you for making that change.)  What I'm 
saying now is mostly just that the comments are misleading.  I suggest just 
removing the comments within the "if".  The code is understandable without them.

Also, this bit of code uses tabs to indent when none of the rest does.  Please 
be consistent with the surrounding code.

Thanks,
Ken

Re: macdrv: implement getting and setting the screen saver state.

[Ken Thomases]

On Jan 20, 2013, at 11:47 AM, C.W. Betts wrote:

> On Jan 20, 2013, at 12:38 AM, Ken Thomases  wrote:
> 
>> On Jan 19, 2013, at 4:08 PM, C.W. Betts wrote:
>> 
>>> +CFNumberRef count = 
>>> CFDictionaryGetValue(assertsionStats, kIOPMAssertionTypeNoDisplaySleep);
>>> +CFNumberRef count2 = NULL;
>>> +long longCount = 0;
>>> +long longCount2 = 0;
>>> +CFNumberGetValue(count, kCFNumberLongType, &longCount);
>>> +count2 = CFDictionaryGetValue(assertsionStats, 
>>> kIOPMAssertionTypePreventUserIdleDisplaySleep);
>>> +if(count2)
>>> +CFNumberGetValue(count2, kCFNumberLongType, 
>>> &longCount2);
>> 
>> Why are the two assertion types handled differently here?  You get the value 
>> in the initializer in one case but not in the other.  You check if the 
>> CFNumber pointer is NULL in one case but not the other.  They should 
>> probably both be handled the same way.
> On Lion and later, kIOPMAssertionTypeNoDisplaySleep is deprecated in favor of 
> kIOPMAssertionTypePreventUserIdleDisplaySleep. This catches both: I don't 
> know if OS X Lion and later automatically maps 
> kIOPMAssertionTypeNoDisplaySleep to 
> kIOPMAssertionTypePreventUserIdleDisplaySleep, so this takes care of both.

I understand why you're checking both.  My question was why the code wasn't 
similar for checking each case.  You check them both, which is good, but you 
check each in a slightly different way, which is confusing and inconsistent.  
It's also error-prone for the possible future where CFDictionaryGetValue(..., 
kIOPMAssertionTypeNoDisplaySleep) might return NULL.  Should that happen, 
CFNumberGetValue() would crash.

-Ken

Re: macdrv: implement getting and setting the screen saver state.

[Ken Thomases]

Hi,

On Jan 19, 2013, at 4:08 PM, C.W. Betts wrote:

> This implements getting and setting the screen saver state on the Mac Wine 
> driver.

Thanks for your contribution to the Mac driver.  There are some issues with the 
patch:

> +CFDictionaryRef assertsionStats = NULL;


That variable name has a misspelling.  It should probably be "assertionStates".


> +CFNumberRef count = 
> CFDictionaryGetValue(assertsionStats, kIOPMAssertionTypeNoDisplaySleep);
> +CFNumberRef count2 = NULL;
> +long longCount = 0;
> +long longCount2 = 0;
> +CFNumberGetValue(count, kCFNumberLongType, &longCount);
> +count2 = CFDictionaryGetValue(assertsionStats, 
> kIOPMAssertionTypePreventUserIdleDisplaySleep);
> +if(count2)
> +CFNumberGetValue(count2, kCFNumberLongType, 
> &longCount2);

Why are the two assertion types handled differently here?  You get the value in 
the initializer in one case but not in the other.  You check if the CFNumber 
pointer is NULL in one case but not the other.  They should probably both be 
handled the same way.

Some better variable names would be nice, too.


> +if (longCount + longCount2 > 0)

If think it's better to use "if (longCount || longCount2)".  We're really 
interested in whether or not either is non-zero.  If, somehow, the sum 
overflows or one is a large negative value, we still want to consider that as 
the screen saver being disabled.


> +{
> +*(BOOL *)ptr_param = FALSE;
> +}

It's a minor style preference, but I prefer that single-statement bodies for 
"if"s not have braces.


> +if(success != kIOReturnSuccess)

Another style nitpick: please put a space between "if" and the condition.  That 
applies to the "if(count2)" above, too.


> +/*
> + Introduced in 10.6, not available in 10.5
> +IOReturn success = IOPMAssertionCreateWithName(
> +kIOPMAssertionTypeNoDisplaySleep, 
> kIOPMAssertionLevelOn,
> +CFSTR("Wine Process requesting no screen saver"), 
> &powerAssertion);
> + */

To Alexandre's chagrin, the Mac driver requires 10.6.  So, you might as well 
use the newer function.

> +IOReturn success = 
> IOPMAssertionCreate(kIOPMAssertionTypeNoDisplaySleep,
> +kIOPMAssertionLevelOn, &powerAssertion);

We should use kIOPMAssertionTypePreventUserIdleDisplaySleep instead on OS 
versions where it's available.  We could either check a framework version 
variable (e.g. kCFCoreFoundationVersionNumber) or check if the dictionary 
returned from IOPMCopyAssertionsStatus() contains 
kIOPMAssertionTypePreventUserIdleDisplaySleep as a key.


> +break;

This seems extraneous.

Cheers,
Ken

Re: libwine: Use rpath-based install name and library references for libwine on Mac.

[Ken Thomases]

On Jan 12, 2013, at 4:12 AM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> On Jan 11, 2013, at 11:13 AM, Alexandre Julliard wrote:
>>> It doesn't seem to work when installed:
>>> 
>>> wine: failed to initialize: dlopen(/usr/local/lib/wine/ntdll.dll.so, 258): 
>>> image not found

>> I assume you did indeed build it with the prefix /usr/local and there is a 
>> /usr/local/lib/wine/ntdll.dll.so.  Is that right?
> 
> The prefix is /usr/local, but there are no files in there, I moved them
> somewhere else. That's the whole point, files are supposed to be found
> relative to the binary, not in the hardcoded prefix.

OK, but that still works for me.  I configured with --prefix=$HOME/test1, I 
installed there, and tested.  Then, I renamed ~/test1 to ~/test2 and retested 
and things still worked.

(I'll also point out that none of the Wine library references incorporate 
absolute paths any more with my change.  So, even working when installed in the 
configured prefix dir is an indication that it's successfully finding libwine 
relative to the binary.  It has no other way to find it.)

So, can you provide the output from those commands (as applied to the files 
wherever you've moved them)?

Thanks,
Ken

Re: libwine: Use rpath-based install name and library references for libwine on Mac.

[Ken Thomases]

On Jan 11, 2013, at 11:13 AM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> ---
>> configure.ac  |3 ++-
>> libs/wine/Makefile.in |6 +++---
>> 2 files changed, 5 insertions(+), 4 deletions(-)
> 
> It doesn't seem to work when installed:
> 
> wine: failed to initialize: dlopen(/usr/local/lib/wine/ntdll.dll.so, 258): 
> image not found

It works for me on Mac OS X 10.6.8.  On which version of the OS were you 
testing?

I assume you did indeed build it with the prefix /usr/local and there is a 
/usr/local/lib/wine/ntdll.dll.so.  Is that right?

What output do you get from the following commands:

otool -L /usr/local/bin/wine
otool -L /usr/local/lib/wine/ntdll.dll.so
otool -lV /usr/local/bin/wine | grep -A3 RPATH

Finally, in the development that lead to this patch, I noticed that the quoted 
"failed to initialize" trace only reports the error from the last attempt to 
open ntdll.dll.so.  Can you hack libwine to trace the error from each failed 
attempt?  Something like:

diff --git a/libs/wine/loader.c b/libs/wine/loader.c
index 302e9c1..bb5d8e6 100644
--- a/libs/wine/loader.c
+++ b/libs/wine/loader.c
@@ -830,6 +830,7 @@ void wine_init( int argc, char *argv[], char *error, int 
error_size )
 if (default_dlldir[0] && context.index < nb_dll_paths + 2) 
nb_dll_paths--;
 break;
 }
+else fprintf(stderr, "%s: failed for %s: %s\n", __func__, path, error);
 }
 free_dll_path( &context );
 


-Ken

Re: [PATCH 3/6] loader: On Mac, embed Info.plist in (__TEXT, __info_plist) section.

[Ken Thomases]

On Jan 7, 2013, at 1:23 AM, Per Johansson wrote:

> On Mon, Jan 7, 2013 at 4:48 AM, Charles Davis  wrote:
>> 
>> With this, you won't be able to launch Wine from the Finder or with open(1) 
>> prior to 10.6.
> 
> I don't see how you'd do use open or the Finder to launch wine, could
> you explain more detailed?

Yeah, I'm not sure it's a real problem, but that key should probably not be 
there anyway.  I'll resubmit with that removed.

Thanks for reviewing.

Cheers,
Ken

Re: [PATCH 3/8] winemac: Implement GetDeviceCaps().

[Ken Thomases]

On Dec 17, 2012, at 1:35 PM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> On Dec 17, 2012, at 11:53 AM, Alexandre Julliard wrote:
>> 
>>> Ken Thomases  writes:
>>> 
>>>> Oh, right.  The Mac driver requires 10.6 or later.  So, I'll have to 
>>>> change the configure script to only enable building of it on such systems.
>>> 
>>> Is there really a compelling reason to not support 10.5.8?
>> 
>> There are Cocoa features that aren't available before 10.6.  For example, we 
>> can't change window style masks (whether a window has a title bar, etc.) 
>> after window creation.
> 
> It would be nice if we could still make things work as far as reasonably
> possible. I'm not ready to throw away my Mac Mini just yet...

Funny, I had been under the impression that you were ready to throw it away 
even before you got it. ;)  It will presumably still work with the X11 driver.  
Also, you should be able to upgrade it to 10.6.

Making the Mac driver compatible with 10.5 is not going to be easy.  The window 
style mask thing was just one example of missing Cocoa features.  There are 
others.  There's a separate class of Cocoa APIs for which there are pre-10.6 
equivalents, but I wrote the Mac driver assuming 10.6 and so I'd have to 
rewrite large chunks of it.  (And the 10.5-compatible code would be 
significantly less pleasant to work with.)

It would also be hard for me to test compatibility with 10.5 since I'm running 
10.6.

-Ken

Re: [PATCH 3/8] winemac: Implement GetDeviceCaps().

[Ken Thomases]

On Dec 17, 2012, at 11:53 AM, Alexandre Julliard wrote:

> Ken Thomases  writes:
> 
>> Oh, right.  The Mac driver requires 10.6 or later.  So, I'll have to change 
>> the configure script to only enable building of it on such systems.
> 
> Is there really a compelling reason to not support 10.5.8?

There are Cocoa features that aren't available before 10.6.  For example, we 
can't change window style masks (whether a window has a title bar, etc.) after 
window creation.

-Ken