Skip to content

GitLab

T
tangled
Commit 18f0099a authored by davve@opera.com's avatar davve@opera.com
Browse files
Options

Trigger computation of font size when crossing foreignObject boundary 

When crossing the svg -> html boundary through a foreignObject, the
zoom factor is reset to its initial value. This is because SVG has a
special zoom model where the SVG fragment root puts the scale factor
on the graphics context, in contrast to non-SVG content where each
value is scaled individually.

This means the FontBuilder can't just inherit the font from parent
when crossing a foreignObject boundary. It may have to recompute the
font size because the effective zoom, included in the computed font
size, may change.

Example of behavior after this patch:

<html> <!-- computed font size: 16px. -->
  <body style="zoom: 2"> <!-- computed font size: 32px. -->
    <svg> <!-- scale(2) is applied to the graphics context; computed font size: 32px. -->
      <text>Sample svg text.</text> <!-- svg ignores computed font size to be able to support minimal font size. -->
      <foreignObject> <!-- html rendering again but scale(2) still applied to GC. computed font size must be reset to 16px. -->
        Sample html text. <!-- Text drawn with computed font size 16px but scaled to 32px by the GC -->

In this annotated test, 'Sample html text.' and 'Sample svg text.'
should both be displayed with a 32px font size.

Note: One difference is that the 'sample html text' is not sensitive
to the correct minimal font size since that happens before the scale
on the GC is applied. In order to get proper minimal font size working
on foreignObject, we would probably have to put foreignObject in a
layer separated from the graphical context of the SVG.

BUG=374119

Review URL: https://codereview.chromium.org/308123010

git-svn-id: svn://svn.chromium.org/blink/trunk@175562 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 649efb8f
Hide whitespace changes
Inline Side-by-side
Showing with 46 additions and 25 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 18f0099a
......@@ -1048,6 +1048,11 @@ crbug.com/377013 [ Mac Debug ] fast/repaint/4776765.html [ Pass Failure ]
crbug.com/239722 http/tests/websocket/connection-throttling.html [ Failure ]
crbug.com/374119 svg/zoom/page/zoom-foreignObject.svg [ NeedsRebaseline ]
crbug.com/374119 svg/zoom/text/zoom-hixie-mixed-008.xml [ NeedsRebaseline ]
crbug.com/374119 svg/zoom/text/zoom-hixie-mixed-009.xml [ NeedsRebaseline ]
crbug.com/374119 svg/zoom/text/zoom-foreignObject.svg [ NeedsRebaseline ]
crbug.com/376194 inspector/console/console-format-es6.html [ Skip ]
crbug.com/376194 inspector/console/console-format-es6-symbols-error.html [ Skip ]
crbug.com/376194 inspector/console/console-dir-es6.html [ Skip ]
......
third_party/WebKit/LayoutTests/svg/custom/foreignObject-font-size-on-zoom-expected.html 0 → 100644
View file @ 18f0099a
<!DOCTYPE html>
<body style="margin:0">
<svg width="100%" height="100%">
<foreignObject x="0" y="0" width="100%" height="100%" transform="scale(2)">
Lorem ipsum
</foreignObject>
</svg>
</body>
third_party/WebKit/LayoutTests/svg/custom/foreignObject-font-size-on-zoom.html 0 → 100644
View file @ 18f0099a
<!DOCTYPE html>
<body style="zoom: 2; margin: 0">
<svg width="100%" height="100%">
<foreignObject x="0" y="0" width="100%" height="100%">
Lorem ipsum
</foreignObject>
</svg>
</body>
third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp
View file @ 18f0099a
......@@ -62,18 +62,16 @@ private:
FontBuilder::FontBuilder()
: m_document(0)
, m_useSVGZoomRules(false)
, m_fontSizehasViewportUnits(false)
, m_style(0)
, m_fontDirty(false)
{
}
void FontBuilder::initForStyleResolve(const Document& document, RenderStyle* style, bool useSVGZoomRules)
void FontBuilder::initForStyleResolve(const Document& document, RenderStyle* style)
{
ASSERT(document.frame());
m_document = &document;
m_useSVGZoomRules = useSVGZoomRules;
m_style = style;
m_fontDirty = false;
}
......@@ -530,13 +528,10 @@ void FontBuilder::setSize(FontDescription& fontDescription, float effectiveZoom,
float FontBuilder::getComputedSizeFromSpecifiedSize(FontDescription& fontDescription, float effectiveZoom, float specifiedSize)
{
float zoomFactor = 1.0f;
if (!m_useSVGZoomRules) {
zoomFactor = effectiveZoom;
// FIXME: Why is this here!!!!?!
if (LocalFrame* frame = m_document->frame())
zoomFactor *= frame->textZoomFactor();
}
float zoomFactor = effectiveZoom;
// FIXME: Why is this here!!!!?!
if (LocalFrame* frame = m_document->frame())
zoomFactor *= frame->textZoomFactor();
return FontSize::getComputedSizeFromSpecifiedSize(m_document, zoomFactor, fontDescription.isAbsoluteSize(), specifiedSize);
}
......
third_party/WebKit/Source/core/css/resolver/FontBuilder.h
View file @ 18f0099a
......@@ -43,7 +43,7 @@ public:
FontBuilder();
// FIXME: The name is probably wrong, but matches StyleResolverState callsite for consistency.
void initForStyleResolve(const Document&, RenderStyle*, bool useSVGZoomRules);
void initForStyleResolve(const Document&, RenderStyle*);
void setInitial(float effectiveZoom);
......@@ -112,7 +112,6 @@ private:
float getComputedSizeFromSpecifiedSize(FontDescription&, float effectiveZoom, float specifiedSize);
const Document* m_document;
bool m_useSVGZoomRules;
bool m_fontSizehasViewportUnits;
// FIXME: This member is here on a short-term lease. The plan is to remove
// any notion of RenderStyle from here, allowing FontBuilder to build Font objects
......
third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
View file @ 18f0099a
......@@ -275,11 +275,6 @@ void StyleAdjuster::adjustRenderStyle(RenderStyle* style, RenderStyle* parentSty
if (!(isSVGSVGElement(*e) && e->parentNode() && !e->parentNode()->isSVGElement()))
style->setPosition(RenderStyle::initialPosition());
// RenderSVGRoot handles zooming for the whole SVG subtree, so foreignObject content should
// not be scaled again.
if (isSVGForeignObjectElement(*e))
style->setEffectiveZoom(RenderStyle::initialZoom());
// SVG text layout code expects us to be a block-level style element.
if ((isSVGForeignObjectElement(*e) || isSVGTextElement(*e)) && style->isDisplayInlineType())
style->setDisplay(BLOCK);
......
third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
View file @ 18f0099a
......@@ -645,7 +645,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
}
}
state.fontBuilder().initForStyleResolve(state.document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(state.document(), state.style());
if (element->isLink()) {
state.style()->setIsLink(true);
......@@ -713,7 +713,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForKeyframe(Element* element, const
state.setStyle(RenderStyle::clone(&elementStyle));
state.setLineHeightValue(0);
state.fontBuilder().initForStyleResolve(state.document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(state.document(), state.style());
// We don't need to bother with !important. Since there is only ever one
// decl, there's nothing to override. So just add the first properties.
......@@ -760,7 +760,7 @@ PassRefPtrWillBeRawPtr<AnimatableValue> StyleResolver::createAnimatableValueSnap
{
StyleResolverState state(element.document(), &element);
state.setStyle(&style);
state.fontBuilder().initForStyleResolve(state.document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(state.document(), state.style());
StyleBuilder::applyProperty(property, state, &value);
return CSSAnimatableValueFactory::create(property, style);
}
......@@ -821,7 +821,7 @@ bool StyleResolver::pseudoStyleForElementInternal(Element& element, const Pseudo
state.setParentStyle(RenderStyle::clone(state.style()));
}
state.fontBuilder().initForStyleResolve(state.document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(state.document(), state.style());
// Since we don't use pseudo-elements in any of our quirk/print
// user agent rules, don't waste time walking those rules.
......@@ -890,7 +890,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
ASSERT(rootElementStyle);
state.style()->inheritFrom(rootElementStyle);
state.fontBuilder().initForStyleResolve(state.document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(state.document(), state.style());
PageRuleCollector collector(rootElementStyle, pageIndex);
......@@ -945,7 +945,7 @@ PassRefPtr<RenderStyle> StyleResolver::defaultStyleForElement()
{
StyleResolverState state(document(), 0);
state.setStyle(RenderStyle::create());
state.fontBuilder().initForStyleResolve(document(), state.style(), state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(document(), state.style());
state.style()->setLineHeight(RenderStyle::initialLineHeight());
state.setLineHeightValue(0);
state.fontBuilder().setInitial(state.style()->effectiveZoom());
......@@ -1378,6 +1378,17 @@ void StyleResolver::applyMatchedProperties(StyleResolverState& state, const Matc
applyMatchedProperties<HighPriorityProperties>(state, matchResult, true, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
applyMatchedProperties<HighPriorityProperties>(state, matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);
if (UNLIKELY(isSVGForeignObjectElement(element))) {
// RenderSVGRoot handles zooming for the whole SVG subtree, so foreignObject content should not be scaled again.
//
// FIXME: The following hijacks the zoom property for foreignObject so that children of foreignObject get the
// correct font-size in case of zooming. 'zoom' is part of HighPriorityProperties, along with other font-related
// properties used as input to the FontBuilder, so resetting it here may cause the FontBuilder to recompute the
// font used as inheritable font for foreignObject content. If we want to support zoom on foreignObject we'll
// need to find another way of handling the SVG zoom model.
state.setEffectiveZoom(RenderStyle::initialZoom());
}
if (cachedMatchedProperties && cachedMatchedProperties->renderStyle->effectiveZoom() != state.style()->effectiveZoom()) {
state.fontBuilder().setFontDirty(true);
applyInheritedOnly = false;
......@@ -1454,7 +1465,7 @@ void StyleResolver::applyPropertiesToStyle(const CSSPropertyValue* properties, s
StyleResolverState state(document(), document().documentElement(), style);
state.setStyle(style);
state.fontBuilder().initForStyleResolve(document(), style, state.useSVGZoomRules());
state.fontBuilder().initForStyleResolve(document(), style);
for (size_t i = 0; i < count; ++i) {
if (properties[i].value) {
......
third_party/WebKit/Source/core/dom/Document.cpp
View file @ 18f0099a
......@@ -1705,7 +1705,7 @@ void Document::updateDistributionForNodeIfNeeded(Node* node)
void Document::setupFontBuilder(RenderStyle* documentStyle)
{
FontBuilder fontBuilder;
fontBuilder.initForStyleResolve(*this, documentStyle, isSVGDocument());
fontBuilder.initForStyleResolve(*this, documentStyle);
RefPtrWillBeRawPtr<CSSFontSelector> selector = m_styleEngine->fontSelector();
fontBuilder.createFontForDocument(selector, documentStyle);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment