[Webkit-unassigned] [Bug 81326] Vertical flow support for OpenType fonts with the least platform dependencies

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=81326


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #133975|review?                     |review-
               Flag|                            |




--- Comment #11 from mitz at webkit.org  2012-03-27 10:21:41 PST ---
(From update of attachment 133975)
View in context: https://bugs.webkit.org/attachment.cgi?id=133975&action=review

Looks good overall. r- because of the potential out-of-bounds reads. I also suggest that you use a nested namespace.

> Source/WebCore/platform/graphics/opentype/OpenTypeTypes.h:28
> +namespace OpenType {

I’m sorry I wasn’t clear about this when suggesting the namespace. I meant it would be nested in the WebCore namespace. I don’t think we should add top-level namespaces in WebCore.

I also thought that if you introduced a namespace you could drop the OT prefix from most identifiers.

> Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp:37
> +using namespace WebCore;

This shouldn’t be necessary if the namespaces are nested.

> Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp:139
> +        m_advanceWidths[i] = hmtx->entries[i].advanceWidth;

You should check that hmtx is big enough before reading from it (you can’t rely on countHmtxEntries being correct).

> Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp:163
> +                const OTVORGTable::VertOriginYMetrics& metrics = vorg->vertOriginYMetrics[i];

Same here, you can’t trust the table to have as many entries as promised.

> Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp:178
> +        m_advanceHeights[i] = vmtx->entries[i].advanceHeight;

Ditto.

> Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp:194
> +        m_topSideBearings[i] = vmtx->entries[i].topSideBearing;

And again.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.