Wintab dll: Status report.

[Robert North]

Aric Stewart aric-at-codeweavers.com |Wine Mailing Lists| wrote:
> Interesting. Since I had myself as well as at lest 3 other organizations 
> of testers testing this code at each step with Photoshop6 and 7 and 
> Painter 7 as well as a number of test applications. None of us saw these 
> problems.
> 
>>
>> What I found, and could fix:
>>
>>     Sometimes a badly configured tablet can generate
>>     X errors on the XOpenDevice method.
>>     Fixed by ensuring tablet driver was functional.
>>     I don't know if the errors need to be caught, or
>>     not, but I think they were causing crashes.
>>     This is probably related to synchonous/asynchonous
>>     X modes.
> 
> 
> Was XOpenDevice returning a device but also creating errors? That seems 
> really counter intuitive since it is only suppose to return the device 
> if it succeeds. However I am not much of a x11 programmer.
> 
> When this error occurred where was the crash? probably at some point 
> where I am gathering the tablet information. It would be good to find 
> that line and fix it there also.

I will have to re-produce the error first.
That could be difficult: hardware in linux tends to work very well, or
very badly. My tablet is currently working very well.
I could have confused the crash from one of the null pointer errors,
so I'm beginning to suspect this could have been a red herring.


> 
>>
>>     In Photoshop, an infinite loop in method FindOpenContext
>>     when attepting to traverse the wintab context chain.
>>
> 
> Something is going seriously wrong here if there is an infinite loop. 
> The contexts should be a linked list so someone's next is pointing to 
> something previous in the list. WHen i did photoshop it created only 2 
> contexts (and only did anything with 1 of them) both where contexts to 
> the root window.
> 
> Where is this loop being created?

It's actually pretty clear from the code:
The following referes to the "context.c" file....

in fuction TABLET_FindOpenContext, line 129 is:
         ptr = ptr->next;

in function FindOpenContext there should be an equivalent
line, after line 175.

If you still want more information about then this occurs, I can provide it.

> 
>>     In Painter, a number of methods didn't handle NULL prameters
>>     properly, resulting in various crashes.
>>     Wintab uses NULL paramters to signify a query for size of
>>     available data, or to signify a request to flush the Wintab
>>     packet queue.
>>
> 
> Which methods? I thought that i had handled all the cases i had seen 
> used this way.

WTInfoA, fails for standard wCategory, or nIndex values if
lpOutput == NULL.
Fixed by modifying CopyTabletData to suppress memcpy if
target == NULL.

WTPacketsGet,WTPacket,WTPacketsPeek,WTDataGet will all fail when passed
a NULL lpPkts.
I've only really fixed this for WTPacketsGet,as this was the only one
where it caused crashes/
Also another bug I neglected to mention what that many of these
functions flush the buffer using memcpy, to copy overlaping
sections.

> 
>> Aric, I'm wondering if I should really be fixing these things, as
>> these look like the kind of bugs Code Weavers will have fixed in the
>>  normal course of product support.
>>
> 
> Like I said, We saw none of these problems. It was working wonderfully 
> for me, and all our tester and customers. So if you need to fix things 
> for your tablet then you should fix them because I cannot reproduce it 
> here at all.

In that case, I wonder if it could be due to different compiler/library 
versions, and static vs. dynamic linking?


> 
> The first thing you need to do is make sure that your tablet is working 
> under X. There is a great little program called xinput, it is a utility 
> that test the XInput module. You can see all the axis of your device and 
> make sure they are working. I found that to get XInput to recognize 
> pressure from my wacom tablet i needed to run the latest wacom drivers 
> from http://linuxwacom.sourceforge.net/index.php/main.
> 
> you can download xinput from 
> ftp://ftp.x.org/contrib/utilities/xinput-1.2.tar.gz


My tablet works fine with Gimp.
I'll have a look at the latest wacom drivers, and at the XInput
extension.
I'll run the test program as a sanity check.
Infact.... the XInput extension could be what's causing the
scaling to go funny. When I said I was suspicous of the scaling
data from the wacom driver, I really meant XInput. I've been
avoiding patching XInput dll so far.


> 
> 
>> Aric, are these the kind of bugs you were seeing with Painter?
>>
> 
> no actually, the problem I was seeing was a race condition inside 
> painter with the button/pressure testing code. Such that after you 
> lifted the stylus painter would still think that the stylus was in 
> contact with the pad and keep drawing.

Ok, So not there yet :-(

> 
>> I'm suspicious of the Wintab packet building code, as it has to
>> build a C data struct on the fly. Looking at the code though, I can't
>> see anything that could be going wrong.
>> It could also be a scaling issue, where the scales deduced from X
>> are wrong (I remember being suspicious of some of the values returned
>> by the wacom driver).
>>
> 
> This i tested alot with my own test programs and it seemed to be working 
> quite well.
> 
>> I think in the light of the crashing bugs, Aric's patch shouldn't
>> go in as-is.
>>
>> If people want, I could post up a patch with the fixes I've added so
>> far.
>>
> 
> I would like to see more details about where you are having these 
> problems and how you fix them as I may be able to continue to help out.

I think it might be easist for me to post a patch that includes the fixes.
The fixes are pretty simple, really. V.easy to spot with a diff against
you original patch.


Cheers
	-Rob.