Bug 9568 - Please review patch: atsui font backend assumes device/user units are the same.
Summary: Please review patch: atsui font backend assumes device/user units are the same.
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: quartz font backend (show other bugs)
Version: 1.3.9
Hardware: PowerPC Mac OS X (All)
: high normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 9350
  Show dependency treegraph
 
Reported: 2007-01-07 17:53 UTC by Brian Ewins
Modified: 2007-03-24 05:36 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
use correct units throughout. (9.87 KB, patch)
2007-01-07 18:05 UTC, Brian Ewins
Details | Splinter Review
updated patch. (9.59 KB, patch)
2007-01-20 05:30 UTC, Brian Ewins
Details | Splinter Review
removes old_show_glyphs (1/6) (6.75 KB, patch)
2007-03-16 17:37 UTC, Brian Ewins
Details | Splinter Review
refactor CreateSizedCopyOfStyle (2/6) (2.68 KB, patch)
2007-03-16 17:39 UTC, Brian Ewins
Details | Splinter Review
fix lifetime of font size, font matrix (3/6) (1.65 KB, patch)
2007-03-16 17:43 UTC, Brian Ewins
Details | Splinter Review
fix glyph positions (4/6) (1.94 KB, patch)
2007-03-16 17:46 UTC, Brian Ewins
Details | Splinter Review
fix glyph metrics (5/6) (2.34 KB, patch)
2007-03-16 17:47 UTC, Brian Ewins
Details | Splinter Review
patch to text-pattern to show up bug (1.29 KB, patch)
2007-03-16 17:54 UTC, Brian Ewins
Details | Splinter Review
this is the quartz rendering of text-pattern after these patches (693 bytes, image/png)
2007-03-16 18:07 UTC, Brian Ewins
Details
rendering with glyph surfaces (906 bytes, image/png)
2007-03-16 18:17 UTC, Brian Ewins
Details
fix scaling and rotation of quartz text (1.46 KB, patch)
2007-03-18 04:46 UTC, Brian Ewins
Details | Splinter Review

Description Brian Ewins 2007-01-07 17:53:46 UTC
As the summary says, this causes the text-pattern test to show nothing. Patch
follows.
Comment 1 Brian Ewins 2007-01-07 18:05:51 UTC
Created attachment 8328 [details] [review]
use correct units throughout.

This patch could do with some review before I commit it. The main issue is all
the transforms being stored in the _cairo_atsui_font struct, caching them for
later use but also fixes a problem with the old code that it was handing out
pointers to automatic objects (via the ATSUStyle). I've avoided fixing all of
that with this patch, just the matrices.

This patch is to be applied after the ones in bug #9467.
Comment 2 Behdad Esfahbod 2007-01-08 17:00:04 UTC
I'm cluless about the ATSUI stuff.  Vlad?
Comment 3 Brian Ewins 2007-01-20 05:30:12 UTC
Created attachment 8468 [details] [review]
updated patch.

No changes, just bringing patch up-to-date.
Comment 4 Brian Ewins 2007-03-16 17:37:51 UTC
Created attachment 9190 [details] [review]
removes old_show_glyphs (1/6)

Updating the patch so it can be applied again. This time I'm breaking it down into smaller steps so the changes are clearer.

Step 1 is removing old_show_glyphs; theres no point in fixing scaling in a function we're removing.
Comment 5 Brian Ewins 2007-03-16 17:39:41 UTC
Created attachment 9191 [details] [review]
refactor CreateSizedCopyOfStyle (2/6)

Refactor CreateSizedCopyOfStyle, just to simplify the next patch.
Comment 6 Brian Ewins 2007-03-16 17:43:01 UTC
Created attachment 9192 [details] [review]
fix lifetime of font size, font matrix (3/6)

When you set properties on an ATSUStyle, you are responsible for freeing their values (after the ATSUStyle has been freed). This makes the font size and matrix members of atsu_font to ensure that the memory doesn't get scribbled on.
Comment 7 Brian Ewins 2007-03-16 17:46:23 UTC
Created attachment 9193 [details] [review]
fix glyph positions (4/6)

This fixes the positions of glyphs to be in user units, not device units - there's more discussion of this in bug 10067. This patch positions the glyphs a little better than that patch - using the advances results in wider-than-expected string.
Comment 8 Brian Ewins 2007-03-16 17:47:44 UTC
Created attachment 9194 [details] [review]
fix glyph metrics (5/6)

Fixes the glyph metrics to use user units not device units.
Comment 9 Brian Ewins 2007-03-16 17:54:14 UTC
Created attachment 9195 [details] [review]
patch to text-pattern to show up bug

The text-pattern test normally falls back to using the glyph surface rendering on the quartz surface, but its the only test that scales the surface under the text. This just makes the text use a solid fill rather than a pattern so that _cairo_quartz_surface_show_glyphs is used to render the text. It also makes the scaling less severe so that the remaining bug is more obvious. This patch isn't intended for committing at the moment, but a new test like this would be useful.
(Renderings to follow)
Comment 10 Brian Ewins 2007-03-16 18:07:45 UTC
Created attachment 9196 [details]
this is the quartz rendering of text-pattern after these patches

Note that the glyphs are all positioned in the correct places, but are the wrong size. Still something wrong here.
Comment 11 Brian Ewins 2007-03-16 18:17:55 UTC
Created attachment 9197 [details]
rendering with glyph surfaces

for comparison, the image surface rendering, which uses glyph surfaces. Glyph surfaces still need some scaling fixes (see the original patch; I just havent fixed those bits up yet).
Comment 12 Brian Ewins 2007-03-18 04:46:31 UTC
Created attachment 9210 [details] [review]
fix scaling and rotation of quartz text

this fixes up most of the remaining issues with quartz text. The output of the text-rotate test becomes essentially correct, and so does the text-pattern test (patched with the 6/6 patch above so quartz text is actually used). Only one text test has a major problem after this - the font-matrix-translation test. That was broken before these patches and remains broken in the same way.

Still missing one patch from this series: fix up glyph surfaces.
Comment 13 Brian Ewins 2007-03-24 05:36:58 UTC
Patches all applied now, commit d2cdd5eba801fc5f696d1095f237ae53c54b4e2a


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.