[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: Fix for PlgBlt, the XFORM matrix was calculated incorrectly
From: Alexander Almaleh <sashoalm () gmail ! com>
Date: 2014-05-27 20:40:49
Message-ID: CACuAq=ps8Z=wnnxB+YRLa3cUbS0rMhzixeGcta5F8n1qsUeFOQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Ok. Btw, I have some other things on my head now, so it might be a week or
two before I come back to this.
On Fri, May 23, 2014 at 1:35 AM, Michael Mc Donnell <michael@mcdonnell.dk>wrote:
> Hi Alexander
>
> On Thu, May 22, 2014 at 2:25 AM, Alexander Almaleh <sashoalm@gmail.com>wrote:
>>
>> OK, I'll try to write a unit test. I found this tutorial -
>> https://www.winehq.org/docs/winedev-guide/testing, is it the correct
>> thing to read?
>>
>
> Yes that is the correct thing to read.
>
>
>> You might be able to get your patch in without the automated test, but
>>> I think you need to explain clearly what was wrong with the old code and
>>> what you are doing instead. A few more simplified examples that test other
>>> cases in your bug report might also help.
>>>
>>
>> The trouble is I don't quite remember the exact steps I took 5 years ago
>> to come up with the new formula. However, I do remember I came up to it by
>> solving a system of linear equations. Since XFORM matrix is applied to each
>> point, I took 3-4 measurements of how points are transformed by PlgBlt, and
>> then used that to make equations of the type applyXFORM(xSrc) = xDest. By
>> solving the equations for the XFORM matrix, I came up with the correct
>> formula I need to get the XFORM matrix from the input variables. I used an
>> online linear equation solver, so there's no human error.
>>
>> I'll try to recreate the steps, maybe it'll be easier to just explain
>> them. I'll look into making automated tests, too.
>>
>
> Sounds good. Like you are saying, figuring out the formulas again would be
> a good idea. The current code does not explain where the formulas are
> coming from. Adding that as comments in your patch would make it easier to
> verify than the current code.
>
> Michael
>
>
[Attachment #5 (text/html)]
<div dir="ltr">Ok. Btw, I have some other things on my head now, so it might be a \
week or two before I come back to this.</div><div class="gmail_extra"><br><br><div \
class="gmail_quote">On Fri, May 23, 2014 at 1:35 AM, Michael Mc Donnell <span \
dir="ltr"><<a href="mailto:michael@mcdonnell.dk" \
target="_blank">michael@mcdonnell.dk</a>></span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr">Hi Alexander<div><br></div><div \
class="gmail_extra"><div class="gmail_quote"><div class="">On Thu, May 22, 2014 at \
2:25 AM, Alexander Almaleh <span dir="ltr"><<a href="mailto:sashoalm@gmail.com" \
target="_blank">sashoalm@gmail.com</a>></span> wrote:<blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">OK, I'll try to \
write a unit test. I found this tutorial - <a \
href="https://www.winehq.org/docs/winedev-guide/testing" \
target="_blank">https://www.winehq.org/docs/winedev-guide/testing</a>, is it the \
correct thing to read?</div>
</div></div></blockquote><div><br></div></div><div>Yes that is the correct thing to \
read.</div><div class=""><div> </div><blockquote class="gmail_quote" style="margin:0 \
0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr"><div \
class="gmail_extra"> <div class="gmail_quote"><div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr"><div> <div></div></div>
<div>You might be able to get your patch in without the automated test, but I think \
you need to explain clearly what was wrong with the old code and what you are doing \
instead. A few more simplified examples that test other cases in your bug report \
might also help.<br>
</div></div></blockquote><div><br></div></div><div>The trouble is I don't quite \
remember the exact steps I took 5 years ago to come up with the new formula. However, \
I do remember I came up to it by solving a system of linear equations. Since XFORM \
matrix is applied to each point, I took 3-4 measurements of how points are \
transformed by PlgBlt, and then used that to make equations of the type \
applyXFORM(xSrc) = xDest. By solving the equations for the XFORM matrix, I came up \
with the correct formula I need to get the XFORM matrix from the input variables. I \
used an online linear equation solver, so there's no human error.</div>
<div><br></div><div>I'll try to recreate the steps, maybe it'll be easier to \
just explain them. I'll look into making automated tests, \
too.</div></div></div></div></blockquote><div><br></div></div><div>Sounds good. Like \
you are saying, figuring out the formulas again would be a good idea. The current \
code does not explain where the formulas are coming from. Adding that as comments in \
your patch would make it easier to verify than the current code.</div> <span \
class="HOEnZb"><font color="#888888"> \
<div><br></div><div>Michael</div></font></span></div><br></div></div> \
</blockquote></div><br></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic