[PATCH] xdg-shell: add draw states for xdg_surface

Tue May 31 08:53:16 UTC 2016

On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
> Hello Jonas, Mike and Olivier,
> 
> On 30/05/2016 15:09, Olivier Fourdan wrote:
> > Hi Jonas,
> > 
> > ----- Original Message -----
> >> On Mon, May 30, 2016 at 05:01:58AM -0400, Olivier Fourdan wrote:
> >>>
> >>> Do we really want reserved ranges here?
> >>>
> >>> Some people reckon having (undocumented) reserved ranges was a bad idea in
> >>> "states", I wonder if we should redo this here again.
> >>>
> >>> Undocumented states from the reserved range are unlikely to be adopted by
> >>> other desktops, and that might lead to duplication of similar mechanisms
> >>> with different values.
> >>
> >> The purpose of a private range is not to have its values adopted by
> >> other desktop environments, but rather a place to put experimental
> >> things or things that might not suite a proper documented state. The
> >> ranges is intended so that different DE:s don't conflict.
> >>
> >> I don't see what is wrong with that.
> > 
> > I see nothing wrong with it in a separate (optional) protocol for experimenting, but as soon as we have clients and compositors using private values out in the field, it might become harder to put things back into the agreed standard set. 

I don't see why. We either have two integer values meaning the same
thing (the "upstreamed" number, and the private), or we have an
upstreamed integer part of the xdg_ and a private integer part of a
separate protocol.

The "upstreaming" procedure would be identical anyhow: writing a patch
adding the new entry to the enum.

> > 
> 
> Here I agree Olivier for this point, my argue is that xdg intend to
> provide a standard, and anyone expect that every DE follow this
> standard, thus each flags must be well defined and acknowledged. Any
> custom flags must fall in extension, that are not under the umbrella of xdg.

Either way, semantically, the result will be identical. Personally I
don't care much whether per DE state enum entry allocations are to be
the current way or or in an extensions really with its own configure
event. Removing alloctions it from xdg_ would only an inconvenience to
do the exact same thing.

Just to repeat, the purpose of range allocations is to DE:s to make use
of the already present configure/ack_configure and now set_available_...
parts, but with their own private (not part of xdg_) integers, without
the issues of introducing any incompatibilities with other toolkits and
compositors. None of these private entries would ever be added or
referenced in xdg_shell.

> 
> At the moment, You proposal just address the issue of the shadow, thus a
> simple set_no_shadow event should be enough ?

The reason for having a "draw state" besides "window state" (the state
we already have) is so that we have one state that is properly
negotiated. The window state is not negotiated; a compositor will just
add states it thinks the client should know about, but doesn't care much
whether the client changed its drawing in any way.

For draw states, the purpose of adding an entry is for when the
compositor actually cares. Shadows is one such thing, because as far as
I know for example KDE intends to draw its own server side shadows, and
could use a "no shadow" draw state to figure out whether it is possible.

> 
> >> The alternative is to have a separate configure event in an extension.
> >> It would work the same way, is a bit more code to write, but it'd
> >> effectively result in the same problem.
> >>>
> >>>> +      <entry name="no_shadow" value="1" summary="no dropshadow">
> >>>> +        <description summary="the surface without a dropshadow">
> >>>> +          The "no_shadow" draw state implies that the client must not
> >>>> draw
> >>>> +          drop shadow around the surface. This may have side effects
> >>>> +          on usability, e.g., the inability to activate client-initiated
> >>>> +          interactive resize.
> >>>> +        </description>
> >>>> +      </entry>
> >>>> +    </enum>
> >>>
> >>> Is a single "no shadow" state enough, isn't that too restrictive? If we put
> >>> this in perspective with a "tiled" state where we consider having
> >>> tiled_left/right/top/bottom, similarly, we could have "no_shadow_left",
> >>> "no_shadow_right", "no_shadow_top" and ""no_shadow_bottom", that would
> >>> give a finer granularity.
> >>
> >> I think we should just add such tiling states to the window state enum,
> >> rather than the draw state. The only point with putting things in the
> >> draw state enum is to get the negotiation, while tiled vs not tiled is
> >> closer to maximized vs not maximized, i.e. a compositor wouldn't change
> >> its behaviour if the client couldn't not draw itself "tiled" or not.
> > 
> > Sorry, I did not make myself clear, I was not asking for tiling to a be a draw state.
> > 
> >> As discussed in the bug you linked, there might be more to tiling than
> >> shadow as well; it might effect rounded-ness of corners and maybe other
> >> things as well, and adding "no_shadow_right" wouldn't address that.
> > 
> > No I was taking tiling as an example where we'd want the edge, so we might consider having a "no shadow" value per edge as well, but not related to tiling though.
> > 
> > This might come useful for surfaces placed next to the edge of a monitor in a multi-monitor setup for example (so we don't have shadows crossing monitors for example) - Maybe it's just overkill, dunno.
> > 
> > We could also consider "no border", again, per edge as well. Or not.
> >
> 
> This last comments look to overlap with some already existing feature
> within the protocol, I think about the input shape.

Not sure how this has anything to do with input shapes.

Jonas

> 
> Also, because the issue is not well described the discussion overflow to
> severals other issues.
> 
> > Cheers,
> > Olivier
> > 
> > 
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> 
> Best regards
> --
> Benoit Gschwind
> 