[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [2/2 v3] winex11.drv: Implement FlashWindow with _NET_WM_STATE_DEMANDS_ATTENTION.
From: Ken Thomases <ken () codeweavers ! com>
Date: 2015-09-30 15:30:17
Message-ID: C4AFDE65-5117-4EC7-B165-53BD33840D4C () codeweavers ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi,
On Sep 30, 2015, at 9:04 AM, Jactry Zeng <jactry92@gmail.com> wrote:
> 2015-09-30 17:05 GMT+08:00 Ken Thomases <ken@codeweavers.com>:
>
> > Since FlashWindowEx() is the more general function — that is, FlashWindow() could \
> > be fully implemented in terms of FlashWindowEx(), but not the other way around — \
> > shouldn't the driver entry point be FlashWindowEx()? Otherwise, when \
> > FlashWindowEx() is eventually implemented more completely, the driver entry will \
> > have to be changed.
> > For now, since user32 implements FlashWindow() but FlashWindowEx() is a stub (or \
> > semi-stub after your next patch), FlashWindow() would have to create a FLASHWINFO \
> > structure, fill it in, and pass that along. I think that dwFlags would be either \
> > FLASHW_ALL or FLASHW_STOP, depending on bInvert. uCount would be 1, I guess.
> Thanks for your review!
You're welcome!
> Yes, function() will be implemented by functionEx() commonly in Wine.
> But I didn't find a perfect way to implement FlashWindowEx() and then implement \
> FlashWindow() with FlashWindowEx().
I wasn't suggesting that you necessarily implement FlashWindow() in terms of \
FlashWindowEx(). Just that you model the driver entry function after FlashWindowEx() \
instead of FlashWindow(). If the driver doesn't implement the full semantics of \
FlashWindowEx(), that's OK. It's no worse than the semi-stub that FlashWindowEx() \
will be.
> Because in X11, how many times will the window be flashed is depend on design of \
> window manager/desktop environment.(For example, in XFCE, taskbar will always \
> flashes when _NET_WM_STATE_DEMANDS_ATTENTION is sent until the user active the \
> window again; In Ubuntu Unity, it'll only be flashed one time and then highlight \
> taskbar until the user active the window again.) And I don't think it's necessary \
> (and any possible?) to follow Window's design. In this way, implementing uCount and \
> other parameters may be useless, because all we need to do is sending \
> _NET_WM_STATE_DEMANDS_ATTENTION to WM/DE one time when \
> FlashWindow()/FlashWindowEx() is called.
> Similarly, Windows's FlashWindowEx() can decide which part it want to flash - only \
> flash taskbar or only flash caption or flash both. But it'll depend on WM/DE's \
> design in X11.
It's OK for the driver to implement just that subset of the functionality it's able \
to. It's not OK for the interface of the driver to prevent the implementation of the \
full functionality. Don't forget, there are other drivers than X11. The Mac driver \
may have greater flexibility to more fully implement the functionality (or it may \
have less). But if the entry point doesn't provide the information, there's no \
chance for it to do as much as it can.
> So I implemented FlashWindowEx() by calling FlashWindow() simply.
Again, the way the two functions are implemented at the user32 level is a different \
question than what the driver interface is.
> > Why does FlashWindow() only call the driver entry point if the window is \
> > minimized? Shouldn't the driver get the opportunity to act for a non-minimized \
> > window, too?
>
> Same as above, different WM/DE may have different UI effect, that makes it hard to \
> follow Windows's design. In Windows, different UI effects will be showed when \
> window is minimized and non-minimized. And in my opinion, it is a little noisy if \
> USER_Driver->pFlashWindow( hWnd, bInvert ) is called all the time. So I'll prefer \
> to only let the user notice new message when the window is minimized (it may hard \
> to be noticed when it's minimized).
I have the same objection as above. The driver interface should not be designed \
based on the capabilities of one specific driver (or things like WM/DE behavior, \
which are specific to that driver). It has to be general enough for any driver. The \
choices about what the driver actually does should be left to the driver, not user32. \
You're making choices which unnecessarily limit what the driver can choose.
-Ken
[Attachment #5 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;">Hi,<div><br><div><div>On Sep 30, 2015, \
at 9:04 AM, Jactry Zeng <<a \
href="mailto:jactry92@gmail.com">jactry92@gmail.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr"><div \
class="gmail_extra">2015-09-30 17:05 GMT+08:00 Ken Thomases <<a \
href="mailto:ken@codeweavers.com" \
target="_blank">ken@codeweavers.com</a>>:<br></div></div></blockquote><blockquote \
type="cite"><br></blockquote><blockquote type="cite"><div dir="ltr"><blockquote \
type="cite"><div class="gmail_extra">Since FlashWindowEx() is the more general \
function — that is, FlashWindow() could be fully implemented in terms of \
FlashWindowEx(), but not the other way around — shouldn't the driver entry point be \
FlashWindowEx()? Otherwise, when FlashWindowEx() is eventually implemented more \
completely, the driver entry will have to be changed.<br><br>For now, since user32 \
implements FlashWindow() but FlashWindowEx() is a stub (or semi-stub after your next \
patch), FlashWindow() would have to create a FLASHWINFO structure, fill it in, and \
pass that along. I think that dwFlags would be either FLASHW_ALL or \
FLASHW_STOP, depending on bInvert. uCount would be 1, I \
guess.<br></div></blockquote><div class="gmail_extra">Thanks for your \
review!</div></div></blockquote><div><br></div><div>You're \
welcome!</div><br><blockquote type="cite"><div dir="ltr"><div \
class="gmail_extra">Yes, function() will be implemented by functionEx() commonly \
in Wine.</div><div class="gmail_extra">But I didn't find a perfect way to implement \
FlashWindowEx() and then implement FlashWindow() with \
FlashWindowEx().</div></div></blockquote><div><br></div><div>I wasn't suggesting that \
you necessarily implement FlashWindow() in terms of FlashWindowEx(). Just that \
you model the driver entry function after FlashWindowEx() instead of FlashWindow(). \
If the driver doesn't implement the full semantics of FlashWindowEx(), that's \
OK. It's no worse than the semi-stub that FlashWindowEx() will \
be.</div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra">Because \
in X11, how many times will the window be flashed is depend on design of window \
manager/desktop environment.(For example, in XFCE, taskbar will always flashes when \
_NET_WM_STATE_DEMANDS_ATTENTION is sent until the user active the window again; In \
Ubuntu Unity, it'll only be flashed one time and then highlight taskbar until the \
user active the window again.) And I don't think it's necessary (and any possible?) \
to follow Window's design.</div><div class="gmail_extra">In this way, implementing \
uCount and other parameters may be useless, because all we need to do is \
sending _NET_WM_STATE_DEMANDS_ATTENTION to WM/DE one time when \
FlashWindow()/FlashWindowEx() is called.</div></div></blockquote><blockquote \
type="cite"><div dir="ltr"><div class="gmail_extra"><br></div><div \
class="gmail_extra"><div class="gmail_extra">Similarly, Windows's FlashWindowEx() can \
decide which part it want to flash - only flash taskbar or only flash </div><div \
class="gmail_extra">caption or flash both. But it'll depend on WM/DE's design in \
X11.</div></div></div></blockquote><div><br></div><div>It's OK for the driver to \
implement just that subset of the functionality it's able to. It's not OK for \
the interface of the driver to prevent the implementation of the full functionality. \
Don't forget, there are other drivers than X11. The Mac driver may have \
greater flexibility to more fully implement the functionality (or it may have less). \
But if the entry point doesn't provide the information, there's no chance for \
it to do as much as it can.</div><div><br></div><br><blockquote type="cite"><div \
dir="ltr"><div class="gmail_extra"><div class="gmail_extra">So I implemented \
FlashWindowEx() by calling FlashWindow() \
simply.</div></div></div></blockquote><div><br></div><div>Again, the way the two \
functions are implemented at the user32 level is a different question than what the \
driver interface is.</div><div><br></div><br><blockquote type="cite"><div \
dir="ltr"><div class="gmail_extra"></div><div class="gmail_extra"></div><blockquote \
type="cite"><div class="gmail_extra">Why does FlashWindow() only call the driver \
entry point if the window is minimized? Shouldn't the driver get the \
opportunity to act for a non-minimized window, \
too?<br></div></blockquote></div></blockquote><blockquote \
type="cite"><br></blockquote><blockquote type="cite"><div dir="ltr"><div \
class="gmail_extra">Same as above, different WM/DE may have different UI effect, that \
makes it hard to follow Windows's design.</div><div class="gmail_extra">In Windows, \
different UI effects will be showed when window is minimized and \
non-minimized.</div><div class="gmail_extra">And in my opinion, it is a little noisy \
if USER_Driver->pFlashWindow( hWnd, bInvert ) is called all the \
time.</div><div class="gmail_extra">So I'll prefer to only let the user notice new \
message when the window is minimized (it may hard to be noticed when</div><div \
class="gmail_extra">it's minimized).</div></div></blockquote><div><br></div><div>I \
have the same objection as above. The driver interface should not be designed \
based on the capabilities of one specific driver (or things like WM/DE behavior, \
which are specific to that driver). It has to be general enough for any driver. \
The choices about what the driver actually does should be left to the driver, \
not user32. You're making choices which unnecessarily limit what the driver can \
choose.</div></div><br></div><div>-Ken</div><div><br></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic