[Try 2] [3/3] riched20:EM_FORMATRANGE Implementation Patch
Changed C99 style comment to c style. Removed an unnecessary TRACE used for testing of code. The actual patch that brings in the EM_FORMATRANGE function. Tested with programs reported to have problems in Bug 6254. James McKenzie
You accidentally sent the email for try 2 of this patch to wine-devel instead of wine-patches. In function ME_FormatContent in file paint.c you calculate the value
+ numCharPrintable = c.nAvailWidth / (1440/c.dpiTarget.cy); then use it later to set iEndOffset, the return value, when the remainder of the text doesn't fit the page. + if (iEndOffset == -1) + { + iEndOffset = numCharPrintable + 1; + targetBottom = c.pt.y - cyOffset; + } The problem is that numCharPrintable is the width of the page in twips but the return value is supposed to be "the index of the last character that fits in the region, plus 1." So the return value isn't some value in twips, it should be a character index reflecting the page boundary.
As I have mentioned to you before in an offlist email, this will cause problems with applications which try to print all the pages in a loop with cpMax=-1 to specify to print to the end of the text, and cpMin for following calls set to the return value of preview calls to send EM_FORMATRANGE messages. The return value will cause an endless loop for long enough text with more characters than the width of the page in twips. It happens to not break the tests, but that doesn't mean it is correct. On my machine the tests don't even execute that condition. I believe on your machine your machine the condition is taken in the tests, likely because fonts on your machine must be different than on mine. The underlying problem with the correct approach though is that a special case for wrapping which isn't handled properly in wine, so this underlying problem causes the test failure which you are trying to fix the wrong way.
Dylan Smith wrote:
You accidentally sent the email for try 2 of this patch to wine-devel instead of wine-patches.
In function ME_FormatContent in file paint.c you calculate the value
+ numCharPrintable = c.nAvailWidth / (1440/c.dpiTarget.cy);
then use it later to set iEndOffset, the return value, when the remainder of the text doesn't fit the page.
+ if (iEndOffset == -1) + { + iEndOffset = numCharPrintable + 1; + targetBottom = c.pt.y - cyOffset; + }
The problem is that numCharPrintable is the width of the page in twips but the return value is supposed to be "the index of the last character that fits in the region, plus 1." So the return value isn't some value in twips, it should be a character index reflecting the page boundary.
This is how I calculated the value to be the same as that returned by EM_FORMATRANGE when run in Windows(whatever). If you have a better method, that would be great. The first returned value for the first test is 3 in Windows and 3 in Wine. The second return values do not match, as this is 8 in Windows but remains 3 in Wine. I feel this is because we do not retain the truncated string anywhere in the context nor in editor. This will require another 'fix' and that is why I was looking for how to truncate right a UNICODE string as this will affect multi-byte character sets more than single-byte character sets.
As I have mentioned to you before in an offlist email, this will cause problems with applications which try to print all the pages in a loop with cpMax=-1 to specify to print to the end of the text, and cpMin for following calls set to the return value of preview calls to send EM_FORMATRANGE messages. The return value will cause an endless loop for long enough text with more characters than the width of the page in twips.
Hmm. This is not something that I have tested, nor is there a test in riched20 for this. Do you think this should have a test case?
It happens to not break the tests, but that doesn't mean it is correct. On my machine the tests don't even execute that condition.
Interesting. Maybe it is time to go get a distro that still ships on CDs. My Thinkpad cannot read DVD media.
I believe on your machine your machine the condition is taken in the tests, likely because fonts on your machine must be different than on mine. The underlying problem with the correct approach though is that a special case for wrapping which isn't handled properly in wine, so this underlying problem causes the test failure which you are trying to fix the wrong way.
What is the Symbol font that is setup on your machine? I don't know what it is on a Mac, but suspect it is of the Helvetica series. I remember it being a member of the Arial series which is smaller on the y-axis. Also, this code should fire on your system, it is the default when the text line is longer than the print area and no delimiting text is provided. That is the reason the first two test cases exist. James
On Sun, Feb 21, 2010 at 9:33 AM, James McKenzie <jjmckenzie51(a)earthlink.net> wrote:
This is how I calculated the value to be the same as that returned by EM_FORMATRANGE when run in Windows(whatever). If you have a better method, that would be great.
I have a better method, it is returning textlength + 1 in this case, because this is the behaviour that native riched20 has for this case. I have sent you patches plenty of times, so you know what I think is the correct code for this case. Attached is a patch that changes you code to what I think it should be for handling this issue. The wrapping issue should be handled as a separately.
As I have mentioned to you before in an offlist email, this will cause problems with applications which try to print all the pages in a loop with cpMax=-1 to specify to print to the end of the text, and cpMin for following calls set to the return value of preview calls to send EM_FORMATRANGE messages. The return value will cause an endless loop for long enough text with more characters than the width of the page in twips.
Hmm. This is not something that I have tested, nor is there a test in riched20 for this. Do you think this should have a test case?
What I provided was the most severe realistic test case I could think of, and I previously provided you a sample rtf file and a patch that shows the behaviour in wordpad. The automated tests are not very extensive, so I have attached a patch for the automated tests that adds two very simple test cases.
Dylan Smith wrote:
You accidentally sent the email for try 2 of this patch to wine-devel instead of wine-patches.
In function ME_FormatContent in file paint.c you calculate the value
+ numCharPrintable = c.nAvailWidth / (1440/c.dpiTarget.cy);
then use it later to set iEndOffset, the return value, when the remainder of the text doesn't fit the page.
+ if (iEndOffset == -1) + { + iEndOffset = numCharPrintable + 1; + targetBottom = c.pt.y - cyOffset; + }
The problem is that numCharPrintable is the width of the page in twips but the return value is supposed to be "the index of the last character that fits in the region, plus 1." So the return value isn't some value in twips, it should be a character index reflecting the page boundary.
As I have mentioned to you before in an offlist email, this will cause problems with applications which try to print all the pages in a loop with cpMax=-1 to specify to print to the end of the text, and cpMin for following calls set to the return value of preview calls to send EM_FORMATRANGE messages. The return value will cause an endless loop for long enough text with more characters than the width of the page in twips.
Interesting comment here, the long text file that you send me does display properly on my system. I will try with a longer file that caused a major looping problem: A hints file for Quake4 that is about 100 pages long and uses exactly that method to display. The problem may be in the original code's limit on the number of font changes before the code attempts to 'back out' and start over. James McKenzie
participants (2)
-
Dylan Smith -
James McKenzie