Closed Bug 333126 Opened 14 years ago Closed 12 years ago

Bold/Italic text isn't displayed as bold/italic with cairo-gtk2 build if the font is synthesis font

Categories

(Core :: Graphics, defect, P2)

x86
Linux
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: hidenosuke, Assigned: masayuki)

References

Details

(Keywords: intl, jp-critical, Whiteboard: cairo)

Attachments

(11 files, 4 obsolete files)

131.72 KB, image/png
Details
132.10 KB, image/png
Details
47.87 KB, image/png
Details
41.74 KB, image/png
Details
308 bytes, patch
pavlov
: review+
damons
: approval1.9+
Details | Diff | Splinter Review
1.43 KB, application/xhtml+xml
Details
43.29 KB, image/png
Details
38.34 KB, image/png
Details
38.16 KB, image/png
Details
25.51 KB, patch
Details | Diff | Splinter Review
1.64 KB, patch
karlt
: review+
Details | Diff | Splinter Review
Japenese fonts aren't display as bold with cairo-gtk2 build.
Non-cairo build doesn't have this problem.
Plese see screen shot.
This happens not only Japanese fonts.
It seems on cairo-gtk2 build em-bold doesn't work anymore.
See screen shots of attachment 215304 [details].
Attached image Screenshot (2006040604)
Screenshot on 2006040604 trunk.

I have Lucida Grande font (only Regular weight).
And I don't have Lucida Sans Unicode font.

Font setting is default(new) profile's one.
Screenshot on 2006040704 trunk cairo-gtk2.

I have Lucida Grande font (only Regular weight).
And I don't have Lucida Sans Unicode font.

Font setting is default(new) profile's one.
This seems to be the Linux equivalent of bug 324611 which existed on Windows but was fixed. Attachment 215304 [details] is not really relevant because it's designed to test a different bug. A better test would be attachment 213305 [details].
Whiteboard: cairo
(In reply to comment #6)
> This seems to be the Linux equivalent of bug 324611 which existed on Windows
> but was fixed. Attachment 215304 [details] [edit] is not really relevant because it's designed
> to test a different bug. A better test would be attachment 213305 [details] [edit].

I can't reproduce with  attachment 213305 [details].
So I don't think this is equivalent of bug 324611.
Summary: Bold Japanese fonts aren't display as bold with cairo-gtk2 build → Bold fonts aren't displayed as bold with cairo-gtk2 build
Depends on: 334512
Blocks: 334512
No longer depends on: 334512
Blocks: 334730
No longer blocks: 334512
Probably, this is a cairo issue.

In my environment(Fedora Core 5, I didn't add other fonts.), cairo uses sazanami-gothic for rendering the Japanese characters. But this font(TrueType) file is only regular style font. So there are not separated files of other styles.

I think that cairo_ft_font doesn't support to create other styles from a regular style.
This bug is not only bold style. Italic style has same problem.

CJK font has very many characters. As a matter of fact, nobody can create bold/italic style font. (I don't know such fonts.) We must implement the dynamic style creation for CJK. I'm marking to jp-critical.
Keywords: intl, jp-critical
Summary: Bold fonts aren't displayed as bold with cairo-gtk2 build → Bold/Italic fonts aren't displayed as bold/italic with cairo-gtk2 build
Summary: Bold/Italic fonts aren't displayed as bold/italic with cairo-gtk2 build → Bold/Italic text isn't displayed as bold/italic with cairo-gtk2 build
Blocks: 342946
Now, the FT has |FT_GlyphSlot_Oblique| (for italic style, but only for outline font) and |FT_GlyphSlot_Embolden| (for bold). Cairo is already calls |FT_GlyphSlot_Embolden| at |#if HAVE_FT_GLYPHSLOT_EMBOLDEN| being true.

I think that we should call |FT_GlyphSlot_Oblique| in cairo and we should enable these code by default in Gecko.

Are there problems?
Summary: Bold/Italic text isn't displayed as bold/italic with cairo-gtk2 build → Bold/Italic text isn't displayed as bold/italic with cairo-gtk2 build if the font is synthesis font
Attached patch Patch for enable embolden rv1.0 (obsolete) — Splinter Review
This patch enables the embolden rendering in cairo.
Attachment #249919 - Flags: review?(vladimir)
And the patch only enables on Linux, I don't know the FT of BeOS support the function.
In the original cairo source, the configure script checks whether the system has "FT_GlyphSlot_Embolden" or not.


There are two possible way to the "FT_GlyphSlot_Embolden" and "FT_GlyphSlot_Oblique" detection:

1. Copy the configure (and configure.in?) script from the cairo source and mozilla configure script calls it.

2. imprement the detection mechanism of "FT_GlyphSlot_Embolden" and "FT_GlyphSlot_Oblique" to mozilla configure script.
Anyway, I think we should not set the hard-coded definitions like the attachment 249919 [details] [diff] [review].
Matsuura-san:

What's the "detection mechanism"?

I think that we don't need to support the old version of FT which doesn't support the embolden. Because we *must* support the bold text rendering for CSS.
This is critical for CJT users. (I don't know Korean.) The many fonts use bitmap, so, our languages are not rendered bold/italic style on current trunk. This should block the release.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
I hope that the all linux machines in tinderbox will not be red...
Attachment #249919 - Attachment is obsolete: true
Attachment #260550 - Flags: review+
Firefox:
Linux fxdbug-linux-tbox Dep %
Linux qm-rhel02 dep unit test %

SeaMonkey:
Linux sea-linux-tbox Dep release %

XULRunner:
Linux argo Dep release %

Sunbird:
Linux sb-linux-tbox Dep Lt-Release %
Linux sb-linux-tbox Dep Sb-Release %

are RED by this patch. I'm keep to watch the tinderbox for all Linux machines are start the build with the patch. And after it, I'll backout the patch.
Thunderbird:
Linux tb-linux-tbox Dep release %

Firefox:
Linux fx-linux-tbox Dep %

Backed-out.
Firefox:
Linux argo-vm Dep Nightly %

Maybe, all machines are not have embolden.
Blocks: 377128
This is going to depend on the new ref platform.
Target Milestone: --- → mozilla1.9alpha6
punting remaining a6 bugs to b1, all of these shipped in a5, so we're at least no worse off by doing so.
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1
This patch is required if firefox-trunk is built on x86_64 Fedora 7 (and other distro?).
Attachment #273517 - Flags: review?
Depends on: 327879
(In reply to comment #15)
> What's the "detection mechanism"?

See the attachment in bug 327879 comment 18.
This is a more conventional way to set up preprocessor macro definitions such as HAVE_FT_GLYPHSLOT_EMBOLDEN.
(In reply to comment #24)
> Created an attachment (id=273517) [details]
> add freetype/ftsynth.h
> 
> This patch is required if firefox-trunk is built on x86_64 Fedora 7 (and other
> distro?).

What are the symptoms without this patch?  With the patch in bug 327879, I didn't need the patch in comment #24 (for a --disable-libxul build) on Gentoo x86_64.
(In reply to comment #26)
> What are the symptoms without this patch?  With the patch in bug 327879, I
> didn't need the patch in comment #24 (for a --disable-libxul build) on Gentoo
> x86_64.

You are right if MOZ_ENABLE_LIBXUL is NOT defined.

Attachment 273517 [details] [diff] is required if attachment 260550 [details] [diff] [review] is applied and MOZ_ENABLE_LIBXUL is defined.
I run the configure script without the options related with libxul and then MOZ_ENABLE_LIBXUL is defined on my Fedora 7.
Comment on attachment 273517 [details] [diff] [review]
add freetype/ftsynth.h

Asking vlad to review as I expect we will need this.
Thanks, MATSUURA for the patch.  (My gcc apparently has ac_cv_have_visibility_class_bug=yes so I can't reproduce.)
Attachment #273517 - Flags: review? → review?(vladimir)
Attachment #273517 - Flags: review?(vladimir)
Attachment #273517 - Flags: review+
Attachment #273517 - Flags: approval1.9+
Comment on attachment 273517 [details] [diff] [review]
add freetype/ftsynth.h

Checked this in on trunk.
With the fix for bug 327879, embolden now works appropriately
(tested on http://hidenosuke.org/diary/ and Lucida Sans Unicode part of attachment 215304 [details] - I don't have Lucida Grande), but we still need artificial oblique.

Note that fontconfig sets matrix for artificial oblique.  It would be worth looking at what pangocairo does with this.
Comment on attachment 260550 [details] [diff] [review]
Patch for enable embolden rv1.0.1 (for checkin)

(no longer needed with the checkin for bug 327879)
Attachment #260550 - Attachment is obsolete: true
I confirmed to enable embolden with the fix for bug 32879.

How about the italic?
According to the cairo bugzilla https://bugs.freedesktop.org/show_bug.cgi?id=4509
artifical oblique is also supported by cairo. But I think that's not true.
(In reply to comment #32)
> According to the cairo bugzilla
> https://bugs.freedesktop.org/show_bug.cgi?id=4509
> artifical oblique is also supported by cairo. But I think that's not true.
> 

I'm guessing pangocairo does this through the font_matrix parameter to cairo_scaled_font_create.
are these patches just waiting on someone to land them?
my patch is not needed now. but I don't know Matsuura-san's patch.
My patch has been already applied by pavlov.
thanks, was this bug gone completely?

# I cannot test Linux now...
No. Artificial oblique problem is not fixed.
See comment #32 and comment #33.
the last patch was in fact checked in.  leaving bug open for oblique problem
Priority: -- → P3
Comment on attachment 273517 [details] [diff] [review]
add freetype/ftsynth.h

Resetting approval flags on bugs that have not been checked in by Oct 22, 11:59PM PDT.  Please re-request approval if needed.
Attachment #273517 - Flags: approval1.9+
Comment on attachment 273517 [details] [diff] [review]
add freetype/ftsynth.h

This landed as rev 3.23 of system-headers on 2007-08-20 19:39.
Attachment #273517 - Flags: approval1.9?
Comment on attachment 273517 [details] [diff] [review]
add freetype/ftsynth.h

Re-approving as this bug was already checked in.
Attachment #273517 - Flags: approval1.9? → approval1.9+
Target Milestone: mozilla1.9 M8 → ---
This should work if you're using system cairo and pango_cairo_font_get_scaled_font() (which is currently ifdef'd out).  We might want to consider not making this a blocker since it will be fixed by us enabling that.

the code pangocairo uses is at the bottom of http://svn.gnome.org/viewvc/pango/trunk/pango/pangocairo-fcfont.c?view=markup -- not sure how we do the FcPatternGetMatrix easily without a bunch of extra work but haven't looked at it deeply yet.
Priority: P3 → P5
Attached image screenshot - Firefox 2
The fix of bold problem seems incomplete.

It seems:
    Fonts with boldface:
        All of <b>, <strong>, CSS { font-weight: bold; },
        and CSS { font-weight: bolder; } are displayed as bold.

    Fonts without boldface (synthetic emboldening):
        Only CSS { font-weight: bold; } is displayed as bold.
yeah, right. the "bolder" logic in gfxPangoFont is wrong, probably. It's not checking whether the actual weights are in the font actually.
Assignee: nobody → mozbugz
Blocks: 372404
Moving to wanted list - please re-nom if you think this is a serious issue..
Flags: wanted1.9+
Flags: blocking1.9-
Flags: blocking1.9+
(In reply to comment #50)
> Moving to wanted list - please re-nom if you think this is a serious issue..

font-weight: bold; case is already fixed, so, the most important case was gone. If we cannot fix font-weight: bolder; case, we need to change the our default UA style sheet for <b> and <strong>.
Attached patch Patch for bolder support (obsolete) — Splinter Review
This is a rough sketch of bolder/lighter support. This patch has an invalid approach, but we might need it Gecko1.9.

gfxPangoFonts.cpp has been changed to use comma separated font name list for pango_shape. However, this approach doesn't allow to use different weight for each fonts. So, we can *not* implement the correct bolder/lighter behavior without changing such architecture. But I believe that my approach is enough for most cases...
Mike Schroepfer:

I already created the rough patch for font-weight: bolder/lighter; support. I'll post a final patch in next week. We *MUST* support them on Gecko1.9. Because bolder is used <b> and <strong>. So, only bolder issue should be blocker1.9. (Italic issue should be wanted1.9 for CJK.)
Flags: blocking1.9- → blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: P5 → P2
Attached patch Patch rv0.5 (work in progress) (obsolete) — Splinter Review
This patch also fixes italic/oblique issue! But there are some bugs, please wait a moment.
Assignee: mozbugz → masayuki
Attachment #299143 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Duplicate of this bug: 372404
Attached patch unusable patchSplinter Review
Attachment #299501 - Attachment is obsolete: true
I created a patch (attachment 300711 [details] [diff] [review]), however, it doesn't work fine. Because bug 401988 changes one gfxPangoFont can have two or more font name (comma separated). Therefore this patch cannot resolve the weight with CSS rules. So, I think that our current implementation is wrong for CSS spec. But current implementation can render the bold Japanese text in the testcase on my environment. So, I think there are no blocker issues for Gecko1.9 release in this bug.

Can anyone reproduce the bold (bolder) issue on the testcase (attachment 286508 [details])?
This simple patch fixes only italic/oblique issue.
Attachment #300731 - Flags: review?(pavlov)
(In reply to comment #57)
> ... But current
> implementation can render the bold Japanese text in the testcase on my
> environment.

fontconfig-2.5.0 has this in conf.d/90-synthetic.conf

        <match target="font">
                <!-- check to see if the font is just regular -->
                <test name="weight" compare="less_eq">
                        <const>medium</const>
                </test>
                <!-- check to see if the pattern requests bold -->
                <test target="pattern" name="weight" compare="more">
                        <const>medium</const>
                </test>
                <!--
                  set the embolden flag
                  needed for applications using cairo, e.g. gucharmap, gedit, .
..
                -->
                <edit name="embolden" mode="assign">
                        <bool>true</bool>
                </edit>

which enables the current implementation to render "font-weight: bolder" as bold.

If anyone is not seeing bolder rendered as bold, can they have a look at this file, please?  Perhaps it is comparing against a different weight?

(In reply to comment #58)
> Created an attachment (id=300731) [details]
> Patch for italic v1.0

This looks great, thanks.
Comment on attachment 300731 [details] [diff] [review]
Patch for italic v1.0

karlt:

Stuart recommended you for this reviewer.
Attachment #300731 - Flags: review?(pavlov) → review?(mozbugz)
Comment on attachment 300731 [details] [diff] [review]
Patch for italic v1.0

Thank you.
Attachment #300731 - Flags: review?(mozbugz) → review+
Tsuyoshi SASAMOTO, are you able to check your /etc/fonts/conf.d/90-synthetic.conf, please?

If it differs from comment 59, what distribution do you have?
(In reply to comment #62)
My fontconfig is ver. 2.3.2, which has this section in /etc/fonts/fonts.conf:

	<match target="font">
		<!-- check to see if the font is just regular -->
		<test name="weight" compare="less_eq">
			<int>100</int>
		</test>
		<!-- check to see if the pattern requests bold -->
		<test target="pattern" name="weight" compare="more_eq">
			<int>200</int>
		</test>
		<!-- set the embolden flag -->
		<edit name="embolden" mode="assign">
			<bool>true</bool>
		</edit>
	</match>

After appending this section in ~/.fonts.conf:

	<match target="font">
		<!-- check to see if the font is just regular -->
		<test name="weight" compare="less_eq">
			<const>medium</const>
		</test>
		<!-- check to see if the pattern requests bold -->
		<test target="pattern" name="weight" compare="more">
			<const>medium</const>
		</test>
		<!-- set the embolden flag -->
		<edit name="embolden" mode="assign">
			<bool>true</bool>
		</edit>
	</match>

it works fine with "font-weight: bolder;" (and <b> / <strong>).

It seems the section has changed in fontconfig 2.3.92.
checked-in the italic patch, thanks.
Assignee: masayuki → nobody
Status: ASSIGNED → NEW
Are we good here, then? Or is bold still missing?
(In reply to comment #65)
> Are we good here, then? Or is bold still missing?

Current trunk might depend on the fontconfig settings? Of course it should not be so.
The only thing missing now is font-weight:bolder, <b>, and <strong> with old versions of fontconfig.

The two weights here currently map to semibold and bold.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/gfx/thebes/src/gfxPangoFonts.cpp&rev=1.133&mark=330-331#327

To fix this for older fontconfig versions we'd need to increase 649 to 651, but these two steps (651 and 749) would then both map to bold.

We could call this fixed in fontconfig.

But I do wonder whether we should merge these two values into one anyway as I haven't found a family with both semibold and bold weights.  Merging the two would make <b>A<b>B</b></b> provide a bolder B than A for family "Arial" (if the newer versions of Arial were available).
Ok, downgrading based on that -- ideally, you guys would resolve this bug as fixed, and then file another bug for the old version of fontconfig issue... but I don't think we should even bother fixing that.
Priority: P2 → P4
One landed patch per bug is my new rule of bugzilla life. It's better for live bug management, reopening, and cvs forensics. So, this bug should be fixed and a followup bug filed if it's not WONTFIX as vlad says.

/be
I'll mark this fixed and file then.
Assignee: nobody → masayuki
Priority: P4 → P2
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.