Den 2009-05-04 16:25 skrev Pierre Ossman:
On Mon, 04 May 2009 16:09:24 +0200
Peter Rosin wrote:

Den 2009-05-04 13:49 skrev Pierre Ossman:
 > +The server changes the desktop size by sending a pseudo-rectangle with
 > +the *DesktopSize* pseudo-encoding. The update containing the
 > +pseudo-rectangle must not contain any rectangles that change the
 > +framebuffer data.

I think "must not" is out of line in this paragraph, you even describe
servers not doing it that way in the ending notes... The correct thing
to say is "should not" IMHO.


Perhaps. I could only find a single server that implements it this way
though, so I was hoping a strong wording would avoid any more. :)

That doesn't solve anything though. I would like my implementation to
be compatible with both the spec and the wilderness. Tightening the
spec after the fact does not achieve that goal.

 > +Also note that some servers send framebuffer data changes before the
 > +*DesktopSize* pseudo-rectangle. A client that wishes to be compatible
 > +with these servers must either retain the framebuffer contents as the
 > +dimensions change, or make sure that the next
 > +*FrameBufferUpdateRequest* has *incremental* set to zero.

Are you really certain about how you should interpret the fb-changing
rects leading up to the DesktopSize rect? There are bound to exist
servers that in some cases push out some pending totally superfluous
"dirty" rects before a final DesktopSize rect in an update. Heck, if you
combine with LastRect, the server might not even be aware of the fact
that it is about to send a DesktopSize rect at the end of the update
when it starts to generate it.

I suppose there is some value to allow harmless noise before the
DesktopSize, but there is a risk that we'll see more people implement
it like RealVNC. See below...

In my reading of the spec, I have always thought that those initial
rects where supposed to be applied before the change in desktop size,
but maybe that's just me. I wouldn't bet on that though.


That would make sense, wouldn't it. :)

Unfortunately that's not how RealVNC is implemented. It will send a
bunch of updates followed by the DesktopSize, when those updates
actually happened after the server resized the framebuffer.

This is incompatible with all clients derived from Tight (which is the
original implementation of this extension AFAICT) which throw away the
framebuffer upon seeing a DesktopSize.

Aaarghh, the RealVNC interpretation is really insane. You then have to
wait for the whole update and check if the last rect is a DesktopSize
rect before you start taking any other action (either that, or you need
some way to backtrack what you just did and then redo it after changing
FB size when you discover a DesktopSize rect, good luck with that...).

Buttom line is, the only sensible thing for a client to do in response
to a DesktopSize rect is to always make a non-incremental FBU request
in order to not rely on some server-specific quirk. It can't know if
the server has retained the fb or not, so it has to make the non-incr
FBU request to get to a known state.

Only UltraVNC sends a non-incr FUR. RealVNC, Tight and gtk-vnc all
continue to send incremental FUR (from what I can tell from my reading
of the code at least, feel free to object if this is wrong).

Unless we want a convoluted specification where the requirements on the
server and client do not match up in a symmetrical manner, we need to
select on one model and declare the other one a buggy exception. There
is the RealVNC model where the client keeps the framebuffer, or the
Tight model where the client throws it away. The latter is the simpler
and more common one.

Here you are not making sense. First you say that UltraVNC is the only
one to send non-incr, so the others you mention should be sending incr.
But then you say that in the tight model the client throws the FB
away, but the only way to do that AFAICT is to send a non-incr FBU
request. So, which way is it?

Anyway, do you really think RealVNC is going to change due to something
we write in our forked spec? I would very much like to be compatible
with RealVNC. Same thing with UltraVNC, as much as is possible without
breaking the spec anyway (writing a client that talks to an Ultra server
is ok, but writing a server that can handle the Ultra client is worse).
And I would like the spec to be compatible with existing versions, not
ones that might, if we are lucky, be produced in the future.

So IMO, declaring stuff that exists today and that is compatible with
the original spec as "buggy" is out of the question.

In this case, the (original) spec is so damn unclear that the server
can do almost anything it wants in conjunction with the DesktopSize
rect which leaves the client with a single sane choice, to send a
non-incr FBU request.

We have:

A. One major implementation (RealVNC) that sends the DesktopSize
   after other rects, but out-of-order.
B. At least one minor implementation (display-vnc in libggi) that
   may send DesktopSize after other rects, but in-order.
   However, for the display-vnc case it doesn't really matter since
   the update following the FB resize will be a full update
   regardless of non-incr or incr in the following FBU request.
C. A spec which does not describe A explicitely, instead the sensible
   way to read the spec suggests B, so there are likely(?) other minor
   implementations in class B.

Due to this unfortunate situation, the client must send a non-incr
FBU request after a DesktopSize rect, at least when that rect came
in an update with preceding FB-changing rects.

But you also have this in your proposal:

+The old framebuffer data does not need to be preserved when the
+framebuffer changes size. The server must therefore not use any
+encoding that relies on the previous contents of the framebuffer.

The way I read this is that the server may not assume that the
client has retained any fb data. To me, that has the implication
that the server must send a full update. Since the server has to
do that, the client might as well *always* issue a non-incr FBU
request in response to a DesktopSize rect.

A bonus is that it's a very simple recommendation that is hard
to misinterpret.

Also, a note on this paragraph:

+As some clients send a *FramebufferUpdateRequest* with *incremental*
+set to zero, because of the issue above, the server must not send a new
+*DesktopSize* pseudo-rectangle in the following update. Doing so would
+make the system go into an infinite loop.

I think it's wrong to disallow all back-to-back uses of DesktopSize
rects. What we want to disallow are servers issuing an automatic
DesktopSize rect in response to each and every non-incr FBU request
they see (but that's an ExDesktopSize-ism that doesn't really apply
to plain old DesktopSize).

I think you want the above wording since you want some consistency
with ExDesktopSize. There it is not allowed to respond with a non-incr
FBU request to an ExDesktopSize rect. I think we should keep them
separate (and I further think you should disallow sending DesktopSize
rects to a client that supports both DesktopSize and ExDesktopSize,
allowing both will just add complications without any gain).

I have attached a version of the patch with changes that I would like
(and a diff between the original patch and my patch).

Cheers,
Peter

Index: rfbproto.rst
===================================================================
--- rfbproto.rst        (revision 3799)
+++ rfbproto.rst        (working copy)
@@ -1628,13 +1628,45 @@
 
 A client which requests the *DesktopSize* pseudo-encoding is declaring
 that it is capable of coping with a change in the framebuffer width
-and/or height. The server changes the desktop size by sending a
-pseudo-rectangle with the *DesktopSize* pseudo-encoding as the last
-rectangle in an update. The pseudo-rectangle's *x-position* and
-*y-position* are ignored, and *width* and *height* indicate the new
-width and height of the framebuffer. There is no further data
-associated with the pseudo-rectangle.
+and/or height.
 
+The server changes the desktop size by sending a pseudo-rectangle with
+the *DesktopSize* pseudo-encoding. The update containing the
+pseudo-rectangle should not contain any rectangles that change the
+framebuffer data.
+
+The pseudo-rectangle's *x-position* and *y-position* are ignored, and
+*width* and *height* indicate the new width and height of the
+framebuffer. There is no further data associated with the
+pseudo-rectangle.
+
+The old framebuffer data does not need to be preserved when the
+framebuffer changes size. The server must therefore not use any
+encoding that relies on the previous contents of the framebuffer.
+
+The principle of one framebuffer update being a transition from one
+valid state to another does not hold for updates with the *DesktopSize*
+pseudo-rectangle as the framebuffer contents will temporarily be
+undefined. Clients should try to handle this gracefully, e.g. by
+showing a black framebuffer or delay the screen update until a proper
+update of the framebuffer contents has been received.
+
+Note that some early implementations require the *DesktopSize*
+pseudo-rectangle to be the very last rectangle in an update. Servers
+should make every effort to support these.
+
+Also note that some servers send framebuffer data changes before the
+*DesktopSize* pseudo-rectangle. A client that wishes to be compatible
+with all these servers should make sure that the next
+*FramebufferUpdateRequest* has *incremental* set to zero whenever it
+receives a *DesktopSize* pseudo-rectangle.
+
+As some clients send a *FramebufferUpdateRequest* with *incremental*
+set to zero, because of the issue above, the server must not send a new
+*DesktopSize* pseudo-rectangle as an automatic response to all
+*FramebufferUpdateRequests* with *incremental* set to zero. Doing so
+would make the system go into an infinite loop.
+
 gii (General Input Interface) Pseudo-encoding
 ---------------------------------------------
 
--- ds.patch    2009-05-05 11:40:04.447125000 +0200
+++ ds-peda.patch       2009-05-05 11:43:35.759625000 +0200
@@ -1,8 +1,8 @@
 Index: rfbproto.rst
 ===================================================================
---- rfbproto.rst       (revision 3796)
+--- rfbproto.rst       (revision 3799)
 +++ rfbproto.rst       (working copy)
-@@ -1310,10 +1310,41 @@
+@@ -1628,13 +1628,45 @@
  
  A client which requests the *DesktopSize* pseudo-encoding is declaring
  that it is capable of coping with a change in the framebuffer width
@@ -16,7 +16,7 @@
  
 +The server changes the desktop size by sending a pseudo-rectangle with
 +the *DesktopSize* pseudo-encoding. The update containing the
-+pseudo-rectangle must not contain any rectangles that change the
++pseudo-rectangle should not contain any rectangles that change the
 +framebuffer data.
 +
 +The pseudo-rectangle's *x-position* and *y-position* are ignored, and
@@ -41,12 +41,16 @@
 +
 +Also note that some servers send framebuffer data changes before the
 +*DesktopSize* pseudo-rectangle. A client that wishes to be compatible
-+with these servers must either retain the framebuffer contents as the
-+dimensions change, or make sure that the next
-+*FrameBufferUpdateRequest* has *incremental* set to zero.
++with all these servers should make sure that the next
++*FramebufferUpdateRequest* has *incremental* set to zero whenever it
++receives a *DesktopSize* pseudo-rectangle.
 +
 +As some clients send a *FramebufferUpdateRequest* with *incremental*
 +set to zero, because of the issue above, the server must not send a new
-+*DesktopSize* pseudo-rectangle in the following update. Doing so would
-+make the system go into an infinite loop.
++*DesktopSize* pseudo-rectangle as an automatic response to all
++*FramebufferUpdateRequests* with *incremental* set to zero. Doing so
++would make the system go into an infinite loop.
 +
+ gii (General Input Interface) Pseudo-encoding
+ ---------------------------------------------
+ 
------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image 
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
tigervnc-rfbproto mailing list
tigervnc-rfbproto@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-rfbproto

Reply via email to