Closed
Bug 327184
Opened 19 years ago
Closed 18 years ago
CSS property letter-spacing rendered incorrectly
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: hsumen, Assigned: pavlov)
References
()
Details
(Keywords: testcase)
Attachments
(8 files, 9 obsolete files)
(deleted),
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
pavlov
:
review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060214 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060214 Firefox/1.6a1
testcase - http://pxclassic.net/faces/letter-spacingTest.html
Letter-spacing is not being rendered correctly. Additionally, selecting text that has letter-spacing applied to it shifts the text (incorrectly renders it).
shockwave capture showing this, second line has letter-spacing applied...
http://pxclassic.net/faces/select.swf
Reproducible: Always
Updated•19 years ago
|
Updated•19 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: cairo
Updated•19 years ago
|
OS: Windows XP → All
Comment 1•19 years ago
|
||
This patch fixes on Windows. But it's crashed on justification text. And the letter-spacing width is not correct.
Comment 2•19 years ago
|
||
This patch fixes the problem. But we have another problem that is the error of rounding decimal.I think that we must not round to int in nsThebesFontMetrics. We should round to int just before rendering the string.
Assignee: nobody → masayuki
Attachment #213066 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #213171 -
Flags: review?(vladimir)
Comment 3•19 years ago
|
||
Comment on attachment 213171 [details] [diff] [review]
Patch rv1.1
O.K. I understand the wrong mechanism. I'll create new patch.
Attachment #213171 -
Flags: review?(vladimir) → review-
Comment 4•19 years ago
|
||
This fixes this bug and bug 324870 on Windows. On Mac and Linux(bug 324158), we need more work.
Attachment #213171 -
Attachment is obsolete: true
Attachment #213230 -
Flags: review?(vladimir)
Comment 5•19 years ago
|
||
Sorry. This is better.
Attachment #213230 -
Attachment is obsolete: true
Attachment #213231 -
Flags: review?(vladimir)
Attachment #213230 -
Flags: review?(vladimir)
Comment 6•19 years ago
|
||
We have another problem. The width calculating is wrong on Windows.
Attachment #213231 -
Attachment is obsolete: true
Attachment #213236 -
Flags: superreview?(roc)
Attachment #213236 -
Flags: review?(vladimir)
Attachment #213231 -
Flags: review?(vladimir)
Assignee | ||
Updated•19 years ago
|
Attachment #213236 -
Flags: superreview?(roc) → superreview?(pavlov)
Assignee | ||
Comment 7•19 years ago
|
||
Comment on attachment 213236 [details] [diff] [review]
Patch rv1.4
I checked in the fix to remove the abs() calls on abc.a and abc.c since that fixed 2 or 3 other bugs but was unrelated really to the letter-spacing problems.
I'm going over the rest of this now.
Comment 8•19 years ago
|
||
recreated for latest trunk.
Attachment #213236 -
Attachment is obsolete: true
Attachment #213410 -
Flags: superreview?(pavlov)
Attachment #213410 -
Flags: review?(vladimir)
Attachment #213236 -
Flags: superreview?(pavlov)
Attachment #213236 -
Flags: review?(vladimir)
Updated•19 years ago
|
Assignee: masayuki → nobody
Status: ASSIGNED → NEW
Comment 10•19 years ago
|
||
*** Bug 331882 has been marked as a duplicate of this bug. ***
Comment 11•19 years ago
|
||
*** Bug 332042 has been marked as a duplicate of this bug. ***
Comment 12•19 years ago
|
||
Seems that some other checkin has resolved this bug in Today's build.
Assignee | ||
Comment 13•19 years ago
|
||
(In reply to comment #12)
> Seems that some other checkin has resolved this bug in Today's build.
>
I think you're testing the wrong builds. Nothing has gone in to fix this.
Assignee | ||
Comment 14•19 years ago
|
||
Sorry for taking so long to get to this. I've gone ahead and done this based on the patch from Masayuki. Instead of passing the pointer through to Draw, I've made it call a setter.
Assignee: masayuki → pavlov
Attachment #213410 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #217631 -
Flags: review?(vladimir)
Attachment #213410 -
Flags: superreview?(pavlov)
Attachment #213410 -
Flags: review?(vladimir)
Comment on attachment 217631 [details] [diff] [review]
Patch v2.0
I /think/ this is ok; r=me provided you fix any other platforms if they break!
Attachment #217631 -
Flags: review?(vladimir) → review+
Comment 16•19 years ago
|
||
Stuart:
Hi, thank you for your work.
But you round to dev unit on |nsThebesFontMetrics.cpp|. I think that we should round it on |gfx*Fonts.cpp|. When I select the text that is applied letter-spacing or jsutify, the characters shaken. Please check it.
Comment 17•19 years ago
|
||
You can test on here.
https://bugzilla.mozilla.org/attachment.cgi?id=209763
Comment 18•19 years ago
|
||
Sorry, I misread your patch, comment16 has some wrong things.
But we have some rounding bugs, this patch fixes some problems, but not perfectly. ..
Attachment #217873 -
Flags: review?(pavlov)
Comment 19•19 years ago
|
||
This fixes this bug on Pango. But I can test only on Linux. I cannot test on BeOS. I don't know this patch fixes this bug on BeOS too.
Attachment #218069 -
Flags: review?(pavlov)
Assignee | ||
Comment 20•19 years ago
|
||
Comment on attachment 217873 [details] [diff] [review]
Patch for suppress text shaken on selecting
I'm worried about these changes:
- textrun->Draw(aContext->Thebes(), gfxPoint(NSToIntRound(aX * app2dev), NSToIntRound(aY * app2dev)));
+ textrun->Draw(aContext->Thebes(), gfxPoint(aX * app2dev, aY * app2dev));
We really want these to be rounded -- not rounding them means we'll draw sub-pixel text causing the text to look blurred.
Keeping the rest in floats is probably a good idea though.
I'll play with this some more and see what I can come up with.
Updated•19 years ago
|
Updated•19 years ago
|
Updated•19 years ago
|
Flags: blocking1.9a1?
Comment 21•19 years ago
|
||
Comment on attachment 217873 [details] [diff] [review]
Patch for suppress text shaken on selecting
I found another problem on selection, I think that we need to change nsTextFrame's selection text rendering code for thebes after bug 333659.
Attachment #217873 -
Flags: review?(pavlov)
Comment 22•19 years ago
|
||
Stuart:
Would you hurry up to review the patch for Pango?
Assignee | ||
Updated•19 years ago
|
Attachment #218069 -
Flags: review?(pavlov) → review+
Assignee | ||
Comment 23•19 years ago
|
||
*** Bug 324158 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 24•19 years ago
|
||
*** Bug 335869 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Flags: blocking1.9a1? → blocking1.9a1+
Comment 25•19 years ago
|
||
Attachment #218069 -
Attachment is obsolete: true
Attachment #220204 -
Flags: review+
Updated•19 years ago
|
Attachment #220204 -
Attachment description: Patch rv1.1 for Pango(cehcked-in) → Patch rv1.1 for Pango(checked-in)
Reporter | ||
Comment 26•19 years ago
|
||
(In reply to comment #17)
> You can test on here.
> https://bugzilla.mozilla.org/attachment.cgi?id=209763
>
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060522 Minefield/3.0a1 ID:2006052205 [cairo]
Testcase still shows shifting text. Is there a seperate bug for this?
Comment 27•19 years ago
|
||
(In reply to comment #26)
> (In reply to comment #17)
> > You can test on here.
> > https://bugzilla.mozilla.org/attachment.cgi?id=209763
> >
>
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060522
> Minefield/3.0a1 ID:2006052205 [cairo]
>
> Testcase still shows shifting text. Is there a seperate bug for this?
>
Yeah, but we cannot try to fix it. see comment 21.
Comment 28•18 years ago
|
||
Stuart:
The using mSpacing rendering was broken by bug 340590 on Win32.
But I cannot find the wrong logic, do you have an idea?
Comment 29•18 years ago
|
||
The text overflow from nsTextFrame.
Assignee | ||
Comment 30•18 years ago
|
||
Not sure yet -- Going to look at this and some perf stuff next.
Comment 31•18 years ago
|
||
Stuart:
This patch fixes the regression. But this makes another regression that is some characters width returns wrong value. I don't understand what means the value.
Assignee | ||
Comment 32•18 years ago
|
||
We can't use the 32 scale -- it breaks bitmap fonts because they have a set of specific sizes they support and can't scale like truetype fonts. I don't know why it would help here either. It didn't before...
Comment 33•18 years ago
|
||
I found interesting thing. If opacity is not 1.0, this regression is not reproduced.
Comment 34•18 years ago
|
||
Oops. Sorry, previous testcase is wrong.
Attachment #225615 -
Attachment is obsolete: true
Comment 35•18 years ago
|
||
I think that we lost the accuracy for calculating the position of characters by bug 340590. Therefore, we can look the problem easily. But we have had the shaking problem already. This regression may not be a new one.
Comment 36•18 years ago
|
||
O.K. This patch fixes the regression. We should round the spacing before gfxTextRun::Draw.
Attachment #225711 -
Flags: review?(pavlov)
Comment 37•18 years ago
|
||
This is better. This patch fixes the shaking problem at selecting on Windows(On linux, it's still there.)
Attachment #225711 -
Attachment is obsolete: true
Attachment #225991 -
Flags: review?(pavlov)
Attachment #225711 -
Flags: review?(pavlov)
Comment 38•18 years ago
|
||
(In reply to comment #37)
> (On > linux, it's still there.)
Sorry. This comment is wrong. I confirmed it's fixed on Linux too.
Comment 39•18 years ago
|
||
Stuart:
Would you review the latest patch?
Assignee | ||
Updated•18 years ago
|
Attachment #225991 -
Flags: review?(pavlov) → review+
Comment 40•18 years ago
|
||
checked-in the patch for windows regression.
Assignee | ||
Comment 41•18 years ago
|
||
is this a problem still on linux or mac?
Comment 42•18 years ago
|
||
we don't implement on Mac yet.
http://lxr.mozilla.org/mozilla/source/gfx/thebes/src/gfxAtsuiFonts.cpp#521
Comment 43•18 years ago
|
||
Has patch check-in resulted in regression?
For example, a 9 point font (predominantly, although others) rendering is over-spaced, often overflowing, and a mis-match is noticeable with linked text between the rendered area and the link activation zone.
For example, please see text under "World News Headlines" on the bottom right of the following page, and move mouse over links;
http://www.stuff.co.nz/stuff/0,2106,3716180a12,00.html
Comment 44•18 years ago
|
||
(In reply to comment #43)
> Has patch check-in resulted in regression?
>
> For example, a 9 point font (predominantly, although others) rendering is
> over-spaced, often overflowing, and a mis-match is noticeable with linked text
> between the rendered area and the link activation zone.
>
> For example, please see text under "World News Headlines" on the bottom right
> of the following page, and move mouse over links;
> http://www.stuff.co.nz/stuff/0,2106,3716180a12,00.html
I cannot find the wrong rendering on 2006062904-trunk/Win2k. What's your environment and would you attach a simple testcase or screenshot?
Assignee | ||
Comment 45•18 years ago
|
||
also WFM
Comment 46•18 years ago
|
||
I think this image shows the problem mentioned in comment 43:
http://twpol.dyndns.org/temp/text_links_1.png
Importantly, this only occurs at some sizes - if I Ctrl-- or Ctrl-+, it is fine, and I expect it is rendering a different sized font to that which it is calculating the size from. More than that I don't know.
As the testcase attached to this bug does not have a problem as far as I can make out, I don't think it is necessarily related to this bug at all, but it certainly is broken (build ID 2006-07-03-20).
Comment 47•18 years ago
|
||
(In reply to comment #46)
There are no letter-spacing, word-spacing, justification and small-caps in the screenshot... It is not a this bug.
Comment 48•18 years ago
|
||
(In reply to comment #44)
> (In reply to comment #43)
> > Has patch check-in resulted in regression?
> >
> > For example, a 9 point font (predominantly, although others) rendering is
> > over-spaced, often overflowing, and a mis-match is noticeable with linked text
> > between the rendered area and the link activation zone.
> >
> > For example, please see text under "World News Headlines" on the bottom right
> > of the following page, and move mouse over links;
> > http://www.stuff.co.nz/stuff/0,2106,3716180a12,00.html
>
> I cannot find the wrong rendering on 2006062904-trunk/Win2k. What's your
> environment and would you attach a simple testcase or screenshot?
Example screenshot showing uneven letter spacing, and mismatch between rendered text and active link zone. This example also shows a larger font than that normally affected.
http://img220.imageshack.us/img220/5821/telecom6za.jpg
Comment 49•18 years ago
|
||
(In reply to comment #48)
> (In reply to comment #44)
> > (In reply to comment #43)
> > > Has patch check-in resulted in regression?
> > >
> > > For example, a 9 point font (predominantly, although others) rendering is
> > > over-spaced, often overflowing, and a mis-match is noticeable with linked text
> > > between the rendered area and the link activation zone.
> > >
> > > For example, please see text under "World News Headlines" on the bottom right
> > > of the following page, and move mouse over links;
> > > http://www.stuff.co.nz/stuff/0,2106,3716180a12,00.html
> >
> > I cannot find the wrong rendering on 2006062904-trunk/Win2k. What's your
> > environment and would you attach a simple testcase or screenshot?
>
> Example screenshot showing uneven letter spacing, and mismatch between rendered
> text and active link zone. This example also shows a larger font than that
> normally affected.
>
> http://img220.imageshack.us/img220/5821/telecom6za.jpg
I cannot find the same page of the screenshot. And I cannot find the letter-spacing specified for anchor in the site. (only p.kicks has "letter-spacing: 1px;", but I cannot find the element in some pages.) 'letter-spacing' means the property of CSS. Do you understand?
Please file a new bug. (And also please write a your environments in the new bug, i.e., OS, build ID of Fx, the default font in your settings of Fx. Because I cannot reproduce the bug.)
Assignee | ||
Comment 50•18 years ago
|
||
Yeah, what you're seeing isn't related to letter spacing. I've seen it a couple of times and have some ideas on the problem, but haven't gotten to look in more details yet. Please file a new bug.
Comment 51•18 years ago
|
||
I have some reports. Some linux testers said that this bug is still existing.(the text is moving by selecting.) But I cannot confirm it. I'll ask/confirm to the testers ASAP.
Reporter | ||
Comment 52•18 years ago
|
||
Text-select shifting is rock solid in Windows now, nice work. Can someone confirm for Linux?
Testcase...
https://bugzilla.mozilla.org/show_bug.cgi?id=327184#c17
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060803 Minefield/3.0a1 ID:2006080304 [cairo]
Comment 53•18 years ago
|
||
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060803 Minefield/3.0a1
Yeah, the testcase in attachment 209763 [details] works for me.
Masayuki, have you heard back from any of the Linux testers that reported this wasn't working?
Whiteboard: cairo
Updated•18 years ago
|
Flags: blocking1.9+
Comment 54•18 years ago
|
||
I heard to the reporters, the all reporters cannot reproduce the shaking problem now. But Mac doesn't have the spacing rendering mechanism, but we should mark to FIXED?
Comment 55•18 years ago
|
||
If you believe the issue was fixed by one of the patches in this bug, yes. Otherwise mark it as WORKSFORME.
And yeah, just file another bug for the Mac issue.
Reporter | ||
Comment 56•18 years ago
|
||
This bug should be marked fixed.
Comment 57•18 years ago
|
||
Screenshot of the testcase https://bugzilla.mozilla.org/attachment.cgi?id=225152 on a gtk2 build of Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060827 Minefield/3.0a1.
No text-select shifting though, but text is not displayed after the first space (the first line should be "letter-spacing: 0.12em;"). To reproduce this behavior, use truetype fonts and libfreetype with bytecode interpreter enabled.
Assignee | ||
Comment 58•18 years ago
|
||
this works on windows and linux for me.. file new bugs for any remaining issues.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•