wintab: Incorrectly classifying cursor as stylus instead of eraser

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

wintab: Incorrectly classifying cursor as stylus instead of eraser

Eriks Dobelis-2
Did not attach the patch in my last email :(

For Wacom Pen & Touch Intuos tablet eraser cursor is incorrectly
detected as stylus.
The tablet returns its eraser cursor name as:
Wacom Intuos PT S Pen eraser
and type as:
ERASER

Current code classifies this as STYLUS, because name of the cursor
contains the word 'Pen'. It is very unlikely that a cursor with type
ERASER is STYLUS.

With the patch eraser is correctly detected. Without the patch eraser
works as stylus.

  dlls/winex11.drv/wintab.c | 3 +++
  1 file changed, 3 insertions(+)




0002-Classifying-cursor-as-eraser-if-type-is-eraser.txt (820 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

Alexandre Julliard
Eriks Dobelis <[hidden email]> writes:

> Did not attach the patch in my last email :(
>
> For Wacom Pen & Touch Intuos tablet eraser cursor is incorrectly
> detected as stylus.
> The tablet returns its eraser cursor name as:
> Wacom Intuos PT S Pen eraser
> and type as:
> ERASER
>
> Current code classifies this as STYLUS, because name of the cursor
> contains the word 'Pen'. It is very unlikely that a cursor with type
> ERASER is STYLUS.

There is already a check for eraser type, you should be able to use that
instead.

--
Alexandre Julliard
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

Eriks Dobelis-2
The alternative - just to switch order of checks in
X11DRV_LoadTabletInfo. There is a code fragment:
             /* prefer finding TYPE_PEN(most capable) */
             if (is_stylus(target->name, device_type))
                 cursor.TYPE = CSR_TYPE_PEN;
             else if (is_eraser(target->name, device_type))
                 cursor.TYPE = CSR_TYPE_ERASER;
             else
                 cursor.TYPE = CSR_TYPE_OTHER;

I would need to switch order and check for ERASER first. Problems would
arise if for some device name would be "Pen & Eraser", and type would be
"STYLUS" , and then it would be detected as eraser. In the proposed
patch this problem would not arise as only device is checked for ERASER.
For my specific case both approaches work. Which one you consider as better?

On 2014.03.20. 22:11, Alexandre Julliard wrote:

> Eriks Dobelis <[hidden email]> writes:
>
>> Did not attach the patch in my last email :(
>>
>> For Wacom Pen & Touch Intuos tablet eraser cursor is incorrectly
>> detected as stylus.
>> The tablet returns its eraser cursor name as:
>> Wacom Intuos PT S Pen eraser
>> and type as:
>> ERASER
>>
>> Current code classifies this as STYLUS, because name of the cursor
>> contains the word 'Pen'. It is very unlikely that a cursor with type
>> ERASER is STYLUS.
> There is already a check for eraser type, you should be able to use that
> instead.
>



Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

Alexandre Julliard
Eriks Dobelis <[hidden email]> writes:

> The alternative - just to switch order of checks in
> X11DRV_LoadTabletInfo. There is a code fragment:
>             /* prefer finding TYPE_PEN(most capable) */
>             if (is_stylus(target->name, device_type))
>                 cursor.TYPE = CSR_TYPE_PEN;
>             else if (is_eraser(target->name, device_type))
>                 cursor.TYPE = CSR_TYPE_ERASER;
>             else
>                 cursor.TYPE = CSR_TYPE_OTHER;
>
> I would need to switch order and check for ERASER first. Problems
> would arise if for some device name would be "Pen & Eraser", and type
> would be "STYLUS" , and then it would be detected as eraser. In the
> proposed patch this problem would not arise as only device is checked
> for ERASER. For my specific case both approaches work. Which one you
> consider as better?

Maybe a better way would be to restructure the checks, and give priority
to the type over the name or something like that. The thing is that we
don't want to have an is_stylus function that also checks for erasers,
and then an is_eraser function with slightly different checks.

--
Alexandre Julliard
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

Eriks Dobelis-2
To be able to prioritize we have to change current functions into smth
like this:

get_cursor_type(cursor) { //new function
     can we determine type based on cursor.device? Yes -> return
CSR_TYPE_PEN;
     can we determine type based on cursor.name? Yes -> return
CSR_TYPE_ERASER;
     return CSR_TYPE_OTHER;
}

and rewriting is_stylus  and is_eraser into more simple functions like
is_stylus(cursor) {
   return get_cursor_type(cursor) == CSR_TYPE_PEN;
}


On 2014.03.21. 10:54, Alexandre Julliard wrote:

> Eriks Dobelis <[hidden email]> writes:
>
>> The alternative - just to switch order of checks in
>> X11DRV_LoadTabletInfo. There is a code fragment:
>>              /* prefer finding TYPE_PEN(most capable) */
>>              if (is_stylus(target->name, device_type))
>>                  cursor.TYPE = CSR_TYPE_PEN;
>>              else if (is_eraser(target->name, device_type))
>>                  cursor.TYPE = CSR_TYPE_ERASER;
>>              else
>>                  cursor.TYPE = CSR_TYPE_OTHER;
>>
>> I would need to switch order and check for ERASER first. Problems
>> would arise if for some device name would be "Pen & Eraser", and type
>> would be "STYLUS" , and then it would be detected as eraser. In the
>> proposed patch this problem would not arise as only device is checked
>> for ERASER. For my specific case both approaches work. Which one you
>> consider as better?
> Maybe a better way would be to restructure the checks, and give priority
> to the type over the name or something like that. The thing is that we
> don't want to have an is_stylus function that also checks for erasers,
> and then an is_eraser function with slightly different checks.
>



Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

Michael Stefaniuc-2
On 03/21/2014 10:04 AM, Eriks Dobelis wrote:

> To be able to prioritize we have to change current functions into smth
> like this:
>
> get_cursor_type(cursor) { //new function
>     can we determine type based on cursor.device? Yes -> return
> CSR_TYPE_PEN;
>     can we determine type based on cursor.name? Yes -> return
> CSR_TYPE_ERASER;
>     return CSR_TYPE_OTHER;
> }
>
> and rewriting is_stylus  and is_eraser into more simple functions like
> is_stylus(cursor) {
>   return get_cursor_type(cursor) == CSR_TYPE_PEN;
> }
In that case you could actually get rid of the is_* functions;
"get_cursor_type(cursor) == CSR_TYPE_PEN" is as readable.

bye
        michael

>
>
> On 2014.03.21. 10:54, Alexandre Julliard wrote:
>> Eriks Dobelis <[hidden email]> writes:
>>
>>> The alternative - just to switch order of checks in
>>> X11DRV_LoadTabletInfo. There is a code fragment:
>>>              /* prefer finding TYPE_PEN(most capable) */
>>>              if (is_stylus(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_PEN;
>>>              else if (is_eraser(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_ERASER;
>>>              else
>>>                  cursor.TYPE = CSR_TYPE_OTHER;
>>>
>>> I would need to switch order and check for ERASER first. Problems
>>> would arise if for some device name would be "Pen & Eraser", and type
>>> would be "STYLUS" , and then it would be detected as eraser. In the
>>> proposed patch this problem would not arise as only device is checked
>>> for ERASER. For my specific case both approaches work. Which one you
>>> consider as better?
>> Maybe a better way would be to restructure the checks, and give priority
>> to the type over the name or something like that. The thing is that we
>> don't want to have an is_stylus function that also checks for erasers,
>> and then an is_eraser function with slightly different checks.
>>
>
>
>


--
Michael Stefaniuc                           Tel.: +49-711-96437-199
Supervisor, APAC/EMEA IT Network            Fax.: +49-711-96437-111
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Werner-von-Siemens-Ring 14, 85630 Grasbrunn
Handelsregister: Amtsgericht Muenchen HRB 153243
Geschäftsführer: Mark Hegarty, Charlie Peters, Michael Cunningham,
                 Charles Cachera


Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

John Klehm
Hello,


>To be able to prioritize we have to change current functions into smth like this:

>get_cursor_type(cursor) { //new function
>   can we determine type based on cursor.device? Yes -> return CSR_TYPE_PEN; 
>   can we determine type based on cursor.name? Yes -> return CSR_TYPE_ERASER;
>    return CSR_TYPE_OTHER;
>}

I worked on this code a little bit back in the day, I remember the detection stuff getting weird due to some odd ball drivers (non-wacom stuff doing strange things).

Bug http://bugs.winehq.org/show_bug.cgi?id=11699 and specifically the log http://bugs.winehq.org/attachment.cgi?id=13358 of an Acecad table driver's settings.

Just thought it might be handy to consider if you were to take a shot at rewriting the detection code.

Regards,
--John


On Fri, Mar 21, 2014 at 4:09 AM, Michael Stefaniuc <[hidden email]> wrote:
On 03/21/2014 10:04 AM, Eriks Dobelis wrote:
> To be able to prioritize we have to change current functions into smth
> like this:
>
> get_cursor_type(cursor) { //new function
>     can we determine type based on cursor.device? Yes -> return
> CSR_TYPE_PEN;
>     can we determine type based on cursor.name? Yes -> return
> CSR_TYPE_ERASER;
>     return CSR_TYPE_OTHER;
> }
>
> and rewriting is_stylus  and is_eraser into more simple functions like
> is_stylus(cursor) {
>   return get_cursor_type(cursor) == CSR_TYPE_PEN;
> }
In that case you could actually get rid of the is_* functions;
"get_cursor_type(cursor) == CSR_TYPE_PEN" is as readable.

bye
        michael

>
>
> On <a href="tel:2014.03.21.%2010" value="+12014032110">2014.03.21. 10:54, Alexandre Julliard wrote:
>> Eriks Dobelis <[hidden email]> writes:
>>
>>> The alternative - just to switch order of checks in
>>> X11DRV_LoadTabletInfo. There is a code fragment:
>>>              /* prefer finding TYPE_PEN(most capable) */
>>>              if (is_stylus(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_PEN;
>>>              else if (is_eraser(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_ERASER;
>>>              else
>>>                  cursor.TYPE = CSR_TYPE_OTHER;
>>>
>>> I would need to switch order and check for ERASER first. Problems
>>> would arise if for some device name would be "Pen & Eraser", and type
>>> would be "STYLUS" , and then it would be detected as eraser. In the
>>> proposed patch this problem would not arise as only device is checked
>>> for ERASER. For my specific case both approaches work. Which one you
>>> consider as better?
>> Maybe a better way would be to restructure the checks, and give priority
>> to the type over the name or something like that. The thing is that we
>> don't want to have an is_stylus function that also checks for erasers,
>> and then an is_eraser function with slightly different checks.
>>
>
>
>


--
Michael Stefaniuc                           Tel.: <a href="tel:%2B49-711-96437-199" value="+4971196437199">+49-711-96437-199
Supervisor, APAC/EMEA IT Network            Fax.: <a href="tel:%2B49-711-96437-111" value="+4971196437111">+49-711-96437-111
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Werner-von-Siemens-Ring 14, 85630 Grasbrunn
Handelsregister: Amtsgericht Muenchen HRB 153243
Geschäftsführer: Mark Hegarty, Charlie Peters, Michael Cunningham,
                 Charles Cachera





Reply | Threaded
Open this post in threaded view
|

Re: wintab: Incorrectly classifying cursor as stylus instead of eraser

John Klehm
Found another oddball log: http://www.winehq.org/pipermail/wine-devel/2013-May/099905.html

I don't have any suggestions on what the right solution is... just wanted to make sure you had all the logs.

Regards,
--John


On Sat, Mar 22, 2014 at 12:28 PM, John Klehm <[hidden email]> wrote:
Hello,


>To be able to prioritize we have to change current functions into smth like this:

>get_cursor_type(cursor) { //new function
>   can we determine type based on cursor.device? Yes -> return CSR_TYPE_PEN; 
>   can we determine type based on cursor.name? Yes -> return CSR_TYPE_ERASER;
>    return CSR_TYPE_OTHER;
>}

I worked on this code a little bit back in the day, I remember the detection stuff getting weird due to some odd ball drivers (non-wacom stuff doing strange things).

Bug http://bugs.winehq.org/show_bug.cgi?id=11699 and specifically the log http://bugs.winehq.org/attachment.cgi?id=13358 of an Acecad table driver's settings.

Just thought it might be handy to consider if you were to take a shot at rewriting the detection code.

Regards,
--John


On Fri, Mar 21, 2014 at 4:09 AM, Michael Stefaniuc <[hidden email]> wrote:
On 03/21/2014 10:04 AM, Eriks Dobelis wrote:
> To be able to prioritize we have to change current functions into smth
> like this:
>
> get_cursor_type(cursor) { //new function
>     can we determine type based on cursor.device? Yes -> return
> CSR_TYPE_PEN;
>     can we determine type based on cursor.name? Yes -> return
> CSR_TYPE_ERASER;
>     return CSR_TYPE_OTHER;
> }
>
> and rewriting is_stylus  and is_eraser into more simple functions like
> is_stylus(cursor) {
>   return get_cursor_type(cursor) == CSR_TYPE_PEN;
> }
In that case you could actually get rid of the is_* functions;
"get_cursor_type(cursor) == CSR_TYPE_PEN" is as readable.

bye
        michael

>
>
> On <a href="tel:2014.03.21.%2010" value="+12014032110" target="_blank">2014.03.21. 10:54, Alexandre Julliard wrote:
>> Eriks Dobelis <[hidden email]> writes:
>>
>>> The alternative - just to switch order of checks in
>>> X11DRV_LoadTabletInfo. There is a code fragment:
>>>              /* prefer finding TYPE_PEN(most capable) */
>>>              if (is_stylus(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_PEN;
>>>              else if (is_eraser(target->name, device_type))
>>>                  cursor.TYPE = CSR_TYPE_ERASER;
>>>              else
>>>                  cursor.TYPE = CSR_TYPE_OTHER;
>>>
>>> I would need to switch order and check for ERASER first. Problems
>>> would arise if for some device name would be "Pen & Eraser", and type
>>> would be "STYLUS" , and then it would be detected as eraser. In the
>>> proposed patch this problem would not arise as only device is checked
>>> for ERASER. For my specific case both approaches work. Which one you
>>> consider as better?
>> Maybe a better way would be to restructure the checks, and give priority
>> to the type over the name or something like that. The thing is that we
>> don't want to have an is_stylus function that also checks for erasers,
>> and then an is_eraser function with slightly different checks.
>>
>
>
>


--
Michael Stefaniuc                           Tel.: <a href="tel:%2B49-711-96437-199" value="+4971196437199" target="_blank">+49-711-96437-199
Supervisor, APAC/EMEA IT Network            Fax.: <a href="tel:%2B49-711-96437-111" value="+4971196437111" target="_blank">+49-711-96437-111
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Werner-von-Siemens-Ring 14, 85630 Grasbrunn
Handelsregister: Amtsgericht Muenchen HRB 153243
Geschäftsführer: Mark Hegarty, Charlie Peters, Michael Cunningham,
                 Charles Cachera