cursor maddnes and a solution

cursor maddnes and a solution

[James Simmons]

I still refuse to touch all drivers at a time. Instead I will merge in 
soft_cursor into the internals of the fbcon cursor code. This will make 
soft cursor be longer used. Then we can remove the soft cursor from the
drivers. There will be no soft_cursor wrap around to imageb lit anymore.
I think people can live with that.



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Antonino A. Daplas]

James Simmons wrote:
> I still refuse to touch all drivers at a time. Instead I will merge in 

Why not?  The change is simple, logical and any bugs are easy to spot.

> soft_cursor into the internals of the fbcon cursor code. This will make 

The patch does that already. It's moving softcursor away from fbdev into
fbcon.

> soft cursor be longer used. Then we can remove the soft cursor from the
> drivers.

Yep, the patch does that too.

So, if I distill your arguments, it boils down to that your main concern is
the patch touches all drivers at once.  So you want 1 patch per driver
then? But Documentation/SubmittingPatches says:

	"On the other hand, if you make a single change to numerous files,
	 group those changes into a single patch.  Thus a single logical change
	 is contained within a single patch."

In the end, it seems that you, me and Jon have the same goal in mind. I don't
see any disagreement here.

>There will be no soft_cursor wrap around to imageb lit anymore.
> I think people can live with that.

And how do you propose to implement a softcursor that is not a wrapper to
imageblit?  Using fillrect?  That's fine with me. Just take into consideration
that an XOR Rop needs to read the contents of the framebuffer first before
writing.  And READS are slow on most PCI cards.

What I cannot understand is your aversion against softcursor.  For fbcon,
it works 100% of the time, guaranteed, for any kind of driver, fictional or
otherwise.

===========================================================================
Anyway, I really don't have a strong argument against adding FB_ACCEL_HWCUR
flag to fbdev.  What I'm against is the overloading of info->fbops->fb_cursor
for userspace. I'll try to clarify:

Almost all graphics hardware have support for a cursor, but they are designed
mainly for a mouse cursor, not a text cursor. So in fbdev/fbcon context,
what we have is a mouse cursor emulating a text cursor. The differences are:

1. Movement granularity of a text cursor is per character, mouse cursor is
per pixel. Also mouse cursor must have a low latency (try shaking your mouse)
and movement changes happen at a very fast rate.

2. A mouse cursor usually have a static image.  It does change from time to
time depending on the app. Basically, you load the image once and you can 
choose to forget about it. A text cursor image, on the other hand, changes
depending on the underlying character.

3. A mouse cursor usually has a static color.  A text cursor's color will
change a lot, especially if your editor has syntax highlighting enabled.

4. A mouse cursor is usually steady, a text cursor usually blinks.

5. The size of the cursor is unchanging for both mouse and text.

So we have these 5 characteristics of the cursor:

		text		mouse
-----------------------------------------
position	changing	changing (at a rapid rate)
size		static		static
image		changing	static
color		changing	static
show/hide	changing	static
 
Now, our info->fbops->fb_cursor() accepts a parameter struct fb_cursor which is
more than 100 bytes in size.  It has everything there in order to have a fully
functional cursor.  But, that's a lot of bytes to move.  This is probably okay
for the text cursor because the text cursor characteristics are usually in a
state of flux. For the mouse cursor, on the other hand, only the position is
usually changing.

So that's why I don't want info->fbops->fb_cursor to be used in user space.  It
is inefficient.  Why move 100 bytes of data to move the cursor to x,y? And then
you have to take into consideration that the rate of change is very rapid.

1. So what I want is to have another API for userspace use.  This API needs to
be broken down into something like below:

fb_cursor_move()
fb_cursor_size()
fb_cursor_load_image()
fb_cursor_load_color()
fb_cursor_show()

A typical app will call fb_cursor_move() magnitudes of times more compared
to the rest.  And if fb_cursor_move() accepts only 2 integers, x and y,
we're just moving 8 bytes, in contrast to 100 bytes.

If you think it's too much splitting, we can just split it into two, one for
the characteristics that changes a lot and another for characteristics that
are predominantly static.

Now this need not coexist side by side forever with fbcon's cursor. Once we
have appropriate testing of this new API, we can start porting drivers that
have a working implementation of the old API (there are 5 right now) to the
new one, Once all drivers are ported, we can remove the old API.  With this,
transition from the old API to the new need not be noticed by the general user.

OR 2. We can start overhauling info->fbops->fb_cursor right now, but at the
risk that any bugs will be felt painfully by the user.

OR 3. Let's forget about userspace cursor support.

Tony



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Jon Smirl]

You have a cursor and a caret. The caret is used for typing text and
the cursor follows the mouse.

Next you are are either in text or graphics mode. How you generate the
caret/cursor is quite different in the two modes. User space doesn't
care about text mode so you can handle it in fbconsole. I'm not sure
how gdm generates a mouse cursor in text mode. Since it is fairly easy
to generate the cursor/caret in software in text mode, I don't know it
if it worth the effort to try and use hardware support.

In graphics mode user space will use the hardware mouse cursor if it
is available. I believe there are a few chips that support multiple
sprites, one could be used for the cursor and one for the caret. This
isn't common so the caret is almost always software generated.

The hardware mouse cursor is relevant in graphics mode because you may
not be able to generate screens in real time. If you can't the
software mouse cursor lags and may become unusable. Hardware mouse
cursor eliminates this problem.

Graphical fbconsole doesn't have a lag problem so it may just want to
software generate.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Antonino A. Daplas]

Jon Smirl wrote:
> 
> The hardware mouse cursor is relevant in graphics mode because you may
> not be able to generate screens in real time. If you can't the
> software mouse cursor lags and may become unusable. Hardware mouse
> cursor eliminates this problem.
> 

I don't have anything against hardware cursor support for userspace. And
I don't even mind if we remove hardware cursor support for fbcon (I don't
see the benefit anyway).  What I don't want is using fbops->fb_cursor
to support a userspace hardware cursor.  In it's current incarnation
it is just wrong.  Besides being a multiplexer with a 100-byte parameter,
it also requires information that are internal to fbdev only. And even if
we manage to make it work, we'll end up with a function with a multiple
personality disorder, it behaves differently when client is fbcon, and
changes behavior again when the client is userspace.

So let's keep fbops->fb_cursor for fbcon use only.  Better yet, let fbcon
call soft_cursor unconditionally, remove hardware cursor from all drivers,
then overhaul fbops->fb_cursor. Or if people think this is too radical,
let's create a new cursor API and gradually phase out the old API, as
proposed in my previous post. Here's one:

struct fb_cursor_ops {
	int (*cursor_move)(int x, int y);
	int (*cursor_show)(int show);
	int (*cursor_load_image)(u32 *image, int width, int height);
	int (*cursor_load_color)(struct fb_cmap *cmap);
	int (*cursor_set_size)(int width, int height);
	int (*cursor_capabilities)(int set, int caps);
};

struct fb_info {
	...
	struct fb_cursor_ops *cursor_ops;
	...
};

In cursor_load_image, we can make image to be always 32-bit truecolor and
just let the driver convert it to something the hardware can support.

In cursor_capabilities, it will either set the capabilities, or get it.
int caps may be t0o small to describe the capabilities so this can be changed.

Tony

 


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Antonino A. Daplas]

Antonino A. Daplas wrote:

I believe this is also needed, otherwise we'll be stuck with a
rectangular cursor:

> 
> struct fb_cursor_ops {
>     int (*cursor_move)(int x, int y);
>     int (*cursor_show)(int show);
>     int (*cursor_load_image)(u32 *image, int width, int height);
>     int (*cursor_load_color)(struct fb_cmap *cmap);
>     int (*cursor_set_size)(int width, int height);
>     int (*cursor_capabilities)(int set, int caps);
	int (*cursor_load_mask)(u8 *mask);
> };
> 

Also, if u32 *image is always in 32-bit truecolor, then cursor_load_color
may not be needed.

Tony


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Jon Smirl]

On 8/10/05, Antonino A. Daplas <adaplas@gmail.com> wrote:
> struct fb_cursor_ops {
>         int (*cursor_move)(int x, int y);
>         int (*cursor_show)(int show);
>         int (*cursor_load_image)(u32 *image, int width, int height);
>         int (*cursor_load_color)(struct fb_cmap *cmap);
>         int (*cursor_set_size)(int width, int height);
>         int (*cursor_capabilities)(int set, int caps);
> };

Doen't fb already support an image format with embedded color map? Can
we use it and merge:
>         int (*cursor_load_image)(u32 *image, int width, int height);
>         int (*cursor_load_color)(struct fb_cmap *cmap);

The size can also be computed from the image so the size call can be eliminated.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Andrey Volkov]

Jon Smirl wrote:
> On 8/10/05, Antonino A. Daplas <adaplas@gmail.com> wrote:
> 
>>struct fb_cursor_ops {
>>        int (*cursor_move)(int x, int y);
>>        int (*cursor_show)(int show);
>>        int (*cursor_load_image)(u32 *image, int width, int height);
>>        int (*cursor_load_color)(struct fb_cmap *cmap);
>>        int (*cursor_set_size)(int width, int height);
>>        int (*cursor_capabilities)(int set, int caps);
>>};
> 
> 
> Doen't fb already support an image format with embedded color map? Can
> we use it and merge:
> 
>>        int (*cursor_load_image)(u32 *image, int width, int height);
>>        int (*cursor_load_color)(struct fb_cmap *cmap);
> 
> 
> The size can also be computed from the image so the size call can be eliminated.
> 
You are omit hwd cursor size problem - very few (possible no one)
graphics chips could support cursor with any size. Usually they support
32x32/64x64 pixels cursors only. For kernel fb its have not meaning,
'cause we always could fallback to the soft_cursor (in the fbcon or
directly in a driver), BUT what to do with such crazy user who will wish
to use, ex. 100x100 pix cursor in user space?

-- 
Regards
Andrey Volkov


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Jon Smirl]

On 8/10/05, Andrey Volkov <avolkov@varma-el.com> wrote:
> 
> 
> Jon Smirl wrote:
> > On 8/10/05, Antonino A. Daplas <adaplas@gmail.com> wrote:
> >
> >>struct fb_cursor_ops {
> >>        int (*cursor_move)(int x, int y);
> >>        int (*cursor_show)(int show);
> >>        int (*cursor_load_image)(u32 *image, int width, int height);
> >>        int (*cursor_load_color)(struct fb_cmap *cmap);
> >>        int (*cursor_set_size)(int width, int height);
> >>        int (*cursor_capabilities)(int set, int caps);
> >>};
> >
> >
> > Doen't fb already support an image format with embedded color map? Can
> > we use it and merge:
> >
> >>        int (*cursor_load_image)(u32 *image, int width, int height);
> >>        int (*cursor_load_color)(struct fb_cmap *cmap);
> >
> >
> > The size can also be computed from the image so the size call can be eliminated.
> >
> You are omit hwd cursor size problem - very few (possible no one)
> graphics chips could support cursor with any size. Usually they support
> 32x32/64x64 pixels cursors only. For kernel fb its have not meaning,
> 'cause we always could fallback to the soft_cursor (in the fbcon or
> directly in a driver), BUT what to do with such crazy user who will wish
> to use, ex. 100x100 pix cursor in user space?

You would just get an error back if the hardware cursor doesn't
support the big size. Your user space app would then have to implement
a fallback software one. You always have to implement the software
fallback since not all hardware may expose the cursor.

You can't implement the software cursor for user space in the kernel
because there is no way to synchronize it with user space drawing. I
think the better solution is for user space apps to handle it than to
try and build a synchronization system into fbdev.

If you are writing X apps you don't have to deal with any of this, X
handles it all for you.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Antonino A. Daplas]

Jon Smirl wrote:
> On 8/10/05, Andrey Volkov <avolkov@varma-el.com> wrote:
>>
>> Jon Smirl wrote:
>>> On 8/10/05, Antonino A. Daplas <adaplas@gmail.com> wrote:
>>>
>>>> struct fb_cursor_ops {
>>>>        int (*cursor_move)(int x, int y);
>>>>        int (*cursor_show)(int show);
>>>>        int (*cursor_load_image)(u32 *image, int width, int height);
>>>>        int (*cursor_load_color)(struct fb_cmap *cmap);
>>>>        int (*cursor_set_size)(int width, int height);
>>>>        int (*cursor_capabilities)(int set, int caps);
>>>> };
>>>
>>> Doen't fb already support an image format with embedded color map? Can
>>> we use it and merge:
>>>
>>>>        int (*cursor_load_image)(u32 *image, int width, int height);
>>>>        int (*cursor_load_color)(struct fb_cmap *cmap);

Yes, we can do that.
 
>>>
>>> The size can also be computed from the image so the size call can be eliminated.
>>>
>> You are omit hwd cursor size problem - very few (possible no one)
>> graphics chips could support cursor with any size. Usually they support
>> 32x32/64x64 pixels cursors only. For kernel fb its have not meaning,
>> 'cause we always could fallback to the soft_cursor (in the fbcon or
>> directly in a driver), BUT what to do with such crazy user who will wish
>> to use, ex. 100x100 pix cursor in user space?
> 

If cursor_capabilities() is called, it should return the maximum
dimensions supported by the hardware.  

Meaning, we have to change it to:

int (*cursor_capabilities)(int set, struct fb_cursor_caps *caps);

so it passes struct fb_cursor_caps *caps instead of int caps.

Of course, if user is so crazy that he/she insists on using a 100x100 cursor
image, the kernel should return an error and, as Jon said, fallback to
a software implementation

Tony 


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Andrey Volkov]

Antonino A. Daplas wrote:
> Jon Smirl wrote:
> 
>> On 8/10/05, Andrey Volkov <avolkov@varma-el.com> wrote:
>> You are omit hwd cursor size problem - very few (possible no one)
>> graphics chips could support cursor with any size. Usually they support
>> 32x32/64x64 pixels cursors only. For kernel fb its have not meaning,
>> 'cause we always could fallback to the soft_cursor (in the fbcon or
>> directly in a driver), BUT what to do with such crazy user who will wish
>> to use, ex. 100x100 pix cursor in user space?
>>
>>
> 
> If cursor_capabilities() is called, it should return the maximum
> dimensions supported by the hardware. 
> Meaning, we have to change it to:
> 
> int (*cursor_capabilities)(int set, struct fb_cursor_caps *caps);
> 
> so it passes struct fb_cursor_caps *caps instead of int caps.
> 
And max_depth also must present in struct fb_cursor_caps. Why generate
true_color image for monochrome hwd cursor? Conversion from true_color
to monochrome, IMHO, usually very painful.

-- 
Regards
Andrey Volkov


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[Antonino A. Daplas]

Andrey Volkov wrote:
> Antonino A. Daplas wrote:
>> Jon Smirl wrote:
>>
>>
> And max_depth also must present in struct fb_cursor_caps. Why generate

Yes.

> true_color image for monochrome hwd cursor? Conversion from true_color
> to monochrome, IMHO, usually very painful.

That was just a proposal. Another way, as Jon proposed, is to combine the
image with struct fb_cmap. This is similar to how the penguin logo is
encoded. 

Tony



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[James Simmons]

> I don't have anything against hardware cursor support for userspace. And
> I don't even mind if we remove hardware cursor support for fbcon (I don't
> see the benefit anyway).  

Please don't remove hw cursor support from fbcon :-)

> struct fb_cursor_ops {
> 	int (*cursor_move)(int x, int y);
> 	int (*cursor_show)(int show);
> 	int (*cursor_load_image)(u32 *image, int width, int height);
> 	int (*cursor_load_color)(struct fb_cmap *cmap);
> 	int (*cursor_set_size)(int width, int height);
> 	int (*cursor_capabilities)(int set, int caps);
> };

> struct fb_info {
> 	...
> 	struct fb_cursor_ops *cursor_ops;
> 	...
> };

We don't need all that mess. You just have a struct fb_cursor in struct 
fb_info and fill in what data you want. All call fb_cusor when you want to 
alter the hardware state.



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: cursor maddnes and a solution

[James Simmons]

> In the end, it seems that you, me and Jon have the same goal in mind. I don't
> see any disagreement here.

I like to see it broken up into smaller pieces. 

> And how do you propose to implement a softcursor that is not a wrapper to
> imageblit?  Using fillrect?  That's fine with me. Just take into consideration
> that an XOR Rop needs to read the contents of the framebuffer first before
> writing.  And READS are slow on most PCI cards.
> 
> What I cannot understand is your aversion against softcursor.  For fbcon,
> it works 100% of the time, guaranteed, for any kind of driver, fictional or
> otherwise.

I'm talking about if fillrect is hardware accelerated and imageblit is 
not. In this case we want to use the fillrect.

> ===========================================================================
> Anyway, I really don't have a strong argument against adding FB_ACCEL_HWCUR
> flag to fbdev.  What I'm against is the overloading of info->fbops->fb_cursor
> for userspace. I'll try to clarify:

> Now, our info->fbops->fb_cursor() accepts a parameter struct fb_cursor which is
> more than 100 bytes in size.  It has everything there in order to have a fully
> functional cursor.  But, that's a lot of bytes to move.  This is probably okay
> for the text cursor because the text cursor characteristics are usually in a
> state of flux. For the mouse cursor, on the other hand, only the position is
> usually changing.
>
> So that's why I don't want info->fbops->fb_cursor to be used in user space.  It
> is inefficient.  Why move 100 bytes of data to move the cursor to x,y? And then
> you have to take into consideration that the rate of change is very rapid.

Who said you have to move all that data at one time. We can have a struct 
fb_cursor in struct fb_info like we have for fb_cmap. Fill in default 
data. Fill in the values you want to change. Call fb_cursor fbops when you 
want the changes to take affect. Its not that hard to do.



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf