Implement app_server API to get mouse cursor information

[Wim]

Please refer to #4100 for related information.

I plan to do the work in the following steps:

1. Implement an AS_GET_CURSOR_POSITION command in ServerApp. This function will be modeled after BView::GetMouse() but it will return global coordinates.
2. Implement an AS_GET_CURSOR command in ServerApp. I think that this command should return an archived BCursor object, but since the BCursor::Archive() function has not yet been implemented, I also plan to work on that.
3. Make these two functions available from InterfaceDefs.cpp|h
4. If appropriate, it may also be nice to have a BApplication::GetCursor() function to complement the SetCursor() function.

I will appreciate comments and suggestions.

[stippi]

It's not yet completely clear to me what you need the current cursor for. The rest all sounds like a good plan. With regards to implementing BCursor::Archive(), there are basically two types of cursors. They are either id-based, or they are custom bitmaps. At the moment, custom bitmaps work, the limitations of this format are described in the BeBook. I don't remember if the app_server keeps the original data, if it does, it's probably easy to store the data in a BMessage or transfer it via the link back to the client application. Otherwise, we could add a BCursor constructor which takes an arbitrary BBitmap, and use only that format for archived storage as well.

[Wim]

Replying to stippi:

It's not yet completely clear to me what you need the current cursor for.

I need to cursor so that I can add it or remove it to a screenshot as to allow the user to change the screenshot settings without taking a new screenshot. See comment 4 in ticket #4100, and also the discussion in ticket #3813

[stippi]

Ah ok, that makes sense. I think retrieving it as a BBitmap then could be the most useful. This should be doable, since on the server side, it is just a bitmap.

[Wim]

Replying to stippi:

Ah ok, that makes sense. I think retrieving it as a BBitmap then could be the most useful. This should be doable, since on the server side, it is just a bitmap.

Actually I need the hotspot as well, so I thought it would be better to retrieve an archived BCursor. Another option woulb be to retrieve a BBitmap and a BBpoint. Which option do you think would be better?

[Wim]

Replying to Wim:

Replying to stippi:

Ah ok, that makes sense. I think retrieving it as a BBitmap then could be the most useful. This should be doable, since on the server side, it is just a bitmap.

Actually I need the hotspot as well, so I thought it would be better to retrieve an archived BCursor. Another option would be to retrieve a BBitmap and a BPoint. Which option do you think would be better?

Just now reading the app_server code I realize that tokens are used to pass around bitmaps and cursors. Probably better to use that instead of passing around an archived BCursor.

[stippi]

Agreed on your last points. The patch is missing the corresponding "user land" part. It sounds best to add it to InterfaceDefs.[h|cpp]. I think "get_mouse(BPoint* screenWhere, uint32* buttons)" should be a good signature.

[axeld]

Since a BCursor is not really that helpful, as it doesn't even know its hotspot or bitmap (you can't ask it for it, at least), maybe a BBitmap with a BPoint would make more sense for now.

Alternatively, one could make BCursor more useful, but since this is only a client object, anyway, the server would still only need to provide the above info (eventually with an ID, if possible).

[Wim]

Modified patch to add AS_GET_CURSOR_POSITION command to the app_server, including a corresponding function in InterfaceDefs

[stippi]

Applied with some minor changes in hrev36811. Thanks!

[Wim]

OK, for the next step I need some more advice.

I started working on an AS_GET_CURSOR_BITMAP command that puts the bitmap of the current cursor in the bitmap indicated by a token provided by the caller. It also returns the cursor hotspot as a BPoint.

However, for the caller of the command to provide the bitmap token it needs the size of the cursor before it can create the bitmap. Should I create an additional command AS_GET_CURSOR_SIZE that returns the size of the current cursor? Somehow it doesn't seem to right way to do this. What if the cursor changes between calls to AS_GET_CURSOR_SIZE and AS_GET_CURSOR_BITMAP? Is there a way to prevent the cursor from changing?

Alternatively I can create a new bitmap in the AS_GET_CURSOR_BITMAP command, but in that case I can't just return the token of that bitmap, because I don't see a way for the (userland) caller to get to the bitmap. In that case, do I attach the bitmap in a BMessage? Is this the prefered way?

Another question I have is if it is desirable to have a function in InterfaceDefs for this command. For example, the AS_READ_BITMAP also doesn't have a companion function in InterfaceDefs, but is only handled in the PrivateScreen class. Is there some reason why some commands don't have corresponding functions in InterfaceDefs, or is it just that the implementation of these functions is planned for the future? Should the userland function of this particular command be part of the Screen class, InterfaceDefs, or both?

I am sorry for all the questions, I am just not familiar enough (yet) with Haiku to make these decisions myself.

[stippi]

First of all... asking all these questions is just fine!

ServerBitmaps have a reference count which would need to be increased when there is another client side instance. I think there is no previous example of creating a client side BBitmap from an already existing ServerBitmap. If nothing else, you may need a new private BBitmap constructor which takes care of this. It could indeed be easier to archive the bitmap memory and all other parameters and send it via the link back to the client. It sounds a bit silly, though, since then what happens is that you create a BBitmap on the client side, which in turn creates another ServerBitmap instance and clones the memory. Perhaps a new static BBitmap method BBitmap* BBitmap::CreateFromCursor(const BCursor& cursor) would be the most convenient, since it would have access to all BBitmap private methods you may need or need to create. Axel, what do you think?

[axeld]

Cursors don't use just ServerBitmaps, but its own subclass, so I'm afraid one doesn't get around doing that, at least I don't think adding the CursorBitmaps to an application would work out that good (for those bitmaps, there is always one ServerApp that actually owns the bitmap). I guess it could made work, but it should be way easier to go the seemingly silly road of sending the data to the client, and let it create the BBitmap itself.

Since the link is read one date after the other, you could have code like this (might be pseudo code, it's been a while since I looked at that API):

uint32 size = link.Read<uint32>();
void* data = malloc(size);
if (data == NULL)
	return B_NO_MEMORY;

link.Read(data, size);
// ...
// read other bitmap values, and create bitmap
bitmap->SetBits(data, ...);

[stippi]

Yes, considering that we don't have any mechanism to handle "copy on write" when two BBitmaps share the same data in the server, copying is probably the cleanest solution for now.

[Wim]

Add AS_GET_CURSOR_BITMAP command to the app_server, including a corresponding function in InterfaceDefs. I also cleaned up the ServerApp.cpp file a bit, mainly adding whitespace around case statements.

[Wim]

I got a weird problem that adding data after the cursor bitmap bytes resulted in a B_BAD_VALUE when trying to read them back, and I was not able to figure out why. The patch seems to work OK though, but I could be missing something.

I was thinking that adding a BApplication::GetCursor() function may be a overdoing it a bit, so feel free to close the ticket if this second patch is acceptable. It should not be a lot of work though, so if you still think it is a good idea let me know and I'll work on it.

[stippi]

Patch looks perfect, also thanks for reminding about link.Read() returning the same error on subsequent invokations, I had forgotten about that when working over the other patch.

[stippi]

Applied in hrev36830. Note that the blank lines between case blocks are not required by the coding style, but I realize it improves consistency in the file. Also note we have an AutoDeleter.h header in headers/private/shared (requires UsePrivateHeaders shared ; in the Jamfile). For the case were you need to free malloc()ed data, you can use the MemoryDeleter, if memory serves.