third_party/WebKit/Source/core/paint/ReplacedPainter.cpp - Issue 1817623004: When the SVG element is 'visible', the SVG element is not shown

Side by Side Diff: third_party/WebKit/Source/core/paint/ReplacedPainter.cpp

Issue 1817623004: When the SVG element is 'visible', the SVG element is not shown (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element-expected.html ('k') | no next file » | no next file with comments »

OLD	NEW
1 // Copyright 2014 The Chromium Authors. All rights reserved.	1 // Copyright 2014 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include "core/paint/ReplacedPainter.h"	5 #include "core/paint/ReplacedPainter.h"

6	6

7 #include "core/layout/LayoutReplaced.h"	7 #include "core/layout/LayoutReplaced.h"

8 #include "core/layout/api/SelectionState.h"	8 #include "core/layout/api/SelectionState.h"

9 #include "core/layout/svg/LayoutSVGRoot.h"	9 #include "core/layout/svg/LayoutSVGRoot.h"

10 #include "core/paint/BoxPainter.h"	10 #include "core/paint/BoxPainter.h"

(...skipping 12 matching lines...) Expand all Loading...
23 }	23 }

24	24

25 void ReplacedPainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paint Offset)	25 void ReplacedPainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paint Offset)

26 {	26 {

27 LayoutPoint adjustedPaintOffset = paintOffset + m_layoutReplaced.location();	27 LayoutPoint adjustedPaintOffset = paintOffset + m_layoutReplaced.location();

28 if (!shouldPaint(paintInfo, adjustedPaintOffset))	28 if (!shouldPaint(paintInfo, adjustedPaintOffset))

29 return;	29 return;

30	30

31 LayoutRect borderRect(adjustedPaintOffset, m_layoutReplaced.size());	31 LayoutRect borderRect(adjustedPaintOffset, m_layoutReplaced.size());

32	32

33 if (m_layoutReplaced.hasBoxDecorationBackground() && (paintInfo.phase == Pai ntPhaseForeground \|\| paintInfo.phase == PaintPhaseSelection))	33 if (m_layoutReplaced.style()->visibility() != HIDDEN && m_layoutReplaced.has BoxDecorationBackground() && (paintInfo.phase == PaintPhaseForeground \|\| paintIn fo.phase == PaintPhaseSelection))
	fs 2016/03/23 14:44:23 Ought to be == VISIBLE here (to handle 'collapse' Ought to be == VISIBLE here (to handle 'collapse' which should be the same as 'hidden' in this case.) hyunjunekim2 2016/03/24 12:40:48 Done. Show quoted text On 2016/03/23 14:44:23, fs wrote: > Ought to be == VISIBLE here (to handle 'collapse' which should be the same as > 'hidden' in this case.) Done.
34 m_layoutReplaced.paintBoxDecorationBackground(paintInfo, adjustedPaintOf fset);	34 m_layoutReplaced.paintBoxDecorationBackground(paintInfo, adjustedPaintOf fset);

35	35

36 if (paintInfo.phase == PaintPhaseMask) {	36 if (paintInfo.phase == PaintPhaseMask) {

37 m_layoutReplaced.paintMask(paintInfo, adjustedPaintOffset);	37 m_layoutReplaced.paintMask(paintInfo, adjustedPaintOffset);

38 return;	38 return;

39 }	39 }

40	40

41 if (paintInfo.phase == PaintPhaseClippingMask && (!m_layoutReplaced.hasLayer () \|\| !m_layoutReplaced.layer()->hasCompositedClippingMask()))	41 if (paintInfo.phase == PaintPhaseClippingMask && (!m_layoutReplaced.hasLayer () \|\| !m_layoutReplaced.layer()->hasCompositedClippingMask()))

42 return;	42 return;

43	43

(...skipping 33 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
77 if (paintInfo.phase == PaintPhaseClippingMask) {	77 if (paintInfo.phase == PaintPhaseClippingMask) {

78 BoxPainter(m_layoutReplaced).paintClippingMask(paintInfo, adjust edPaintOffset);	78 BoxPainter(m_layoutReplaced).paintClippingMask(paintInfo, adjust edPaintOffset);

79 } else {	79 } else {

80 m_layoutReplaced.paintReplaced(paintInfo, adjustedPaintOffset);	80 m_layoutReplaced.paintReplaced(paintInfo, adjustedPaintOffset);

81 }	81 }

82 }	82 }

83 }	83 }

84	84

85 // The selection tint never gets clipped by border-radius rounding, since we want it to run right up to the edges of	85 // The selection tint never gets clipped by border-radius rounding, since we want it to run right up to the edges of

86 // surrounding content.	86 // surrounding content.

87 bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layout Replaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting();	87 bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layout Replaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting();
	fs 2016/03/23 14:44:23 Should we paint the selection tint? Given it's a f Should we paint the selection tint? Given it's a foreground thing, probably not. hyunjunekim2 2016/03/24 12:40:48 I guess that we paint this. In such cases, the wil Show quoted text On 2016/03/23 14:44:23, fs wrote: > Should we paint the selection tint? Given it's a foreground thing, probably not. I guess that we paint this. In such cases, the will should be a tint. <!DOCTYPE html> <style type="text/css"> img { width: 160px; height: 100px; } </style> <img src="data:image/svg+xml, <svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16' visibility='hidden'> <rect x='1' y='1' width='14' height='14' fill='lime' visibility='visible'/> </svg>"> hyunjunekim2 2016/03/24 12:47:23 fs, It's different case. fs, It's different case. hyunjunekim2 2016/03/24 13:04:51 On the SVG, when images, It can be selection tint. On the SVG, when images, It can be selection tint. then ignore visible option. hyunjunekim2 2016/03/24 13:12:24 fs, I have question. Is there a case that tint on fs, I have question. Is there a case that tint on the svg document? fs 2016/03/24 13:47:34 You might be able to set one explicitly as a Range Show quoted text On 2016/03/24 at 13:12:24, hyunjunekim2 wrote: > fs, > I have question. Is there a case that tint on the svg document? You might be able to set one explicitly as a Range (via window.selection) hyunjunekim2 2016/03/24 15:31:33 fs, We can't select <svg> element. I ran this tes fs, We can't select <svg> element. I ran this test. but just selected <text>. So i guess that don't care wether visible or not. <!DOCTYPE HTML> <div style="visibility: hidden;"> <svg id="s1" style="background: red;"> <rect id="r1" x="0" y="0" width="100" height="100" fill="green" visibility="visible"></rect> <text x="20" y="20" visibility="visible">hi</text> </svg> </div> <script> var range = document.createRange(); range.selectNode(window.document.documentElement); var selection = window.getSelection(); selection.removeAllRanges(); selection.addRange(range); console.log(selection); </script> fs 2016/03/24 15:39:02 Acknowledged. Show quoted text On 2016/03/24 at 15:31:33, hyunjunekim2 wrote: > fs, > We can't select <svg> element. > I ran this test. but just selected <text>. So i guess that don't care wether > visible or not. > > <!DOCTYPE HTML> > <div style="visibility: hidden;"> > <svg id="s1" style="background: red;"> > <rect id="r1" x="0" y="0" width="100" height="100" fill="green" visibility="visible"></rect> > <text x="20" y="20" visibility="visible">hi</text> > </svg> > </div> > <script> > var range = document.createRange(); > range.selectNode(window.document.documentElement); > var selection = window.getSelection(); > selection.removeAllRanges(); > selection.addRange(range); > console.log(selection); > </script> Acknowledged.
88 if (drawSelectionTint && !LayoutObjectDrawingRecorder::useCachedDrawingIfPos sible(paintInfo.context, m_layoutReplaced, DisplayItem::SelectionTint)) {	88 if (drawSelectionTint && !LayoutObjectDrawingRecorder::useCachedDrawingIfPos sible(paintInfo.context, m_layoutReplaced, DisplayItem::SelectionTint)) {

89 LayoutRect selectionPaintingRect = m_layoutReplaced.localSelectionRect() ;	89 LayoutRect selectionPaintingRect = m_layoutReplaced.localSelectionRect() ;

90 selectionPaintingRect.moveBy(adjustedPaintOffset);	90 selectionPaintingRect.moveBy(adjustedPaintOffset);

91 IntRect selectionPaintingIntRect = pixelSnappedIntRect(selectionPainting Rect);	91 IntRect selectionPaintingIntRect = pixelSnappedIntRect(selectionPainting Rect);

92	92

93 LayoutObjectDrawingRecorder drawingRecorder(paintInfo.context, m_layoutR eplaced, DisplayItem::SelectionTint, selectionPaintingIntRect);	93 LayoutObjectDrawingRecorder drawingRecorder(paintInfo.context, m_layoutR eplaced, DisplayItem::SelectionTint, selectionPaintingIntRect);

94 paintInfo.context.fillRect(selectionPaintingIntRect, m_layoutReplaced.se lectionBackgroundColor());	94 paintInfo.context.fillRect(selectionPaintingIntRect, m_layoutReplaced.se lectionBackgroundColor());

95 }	95 }

96 }	96 }

97	97

98 bool ReplacedPainter::shouldPaint(const PaintInfo& paintInfo, const LayoutPoint& adjustedPaintOffset) const	98 bool ReplacedPainter::shouldPaint(const PaintInfo& paintInfo, const LayoutPoint& adjustedPaintOffset) const

99 {	99 {

100 if (paintInfo.phase != PaintPhaseForeground && !shouldPaintSelfOutline(paint Info.phase)	100 if (paintInfo.phase != PaintPhaseForeground && !shouldPaintSelfOutline(paint Info.phase)

101 && paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPha seMask && paintInfo.phase != PaintPhaseClippingMask)	101 && paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPha seMask && paintInfo.phase != PaintPhaseClippingMask)

102 return false;	102 return false;

103	103

104 // If we're invisible or haven't received a layout yet, just bail.	104 // If we're invisible or haven't received a layout yet, just bail.

105 if (m_layoutReplaced.style()->visibility() != VISIBLE)	105 if (!m_layoutReplaced.isSVGRoot() && m_layoutReplaced.style()->visibility() != VISIBLE)
	fs 2016/03/23 14:44:23 Document the new part of the condition. Document the new part of the condition. hyunjunekim2 2016/03/24 12:40:48 Added the comment. Show quoted text On 2016/03/23 14:44:23, fs wrote: > Document the new part of the condition. Added the comment.
106 return false;	106 return false;

107	107

108 LayoutRect paintRect(m_layoutReplaced.visualOverflowRect());	108 LayoutRect paintRect(m_layoutReplaced.visualOverflowRect());

109 paintRect.unite(m_layoutReplaced.localSelectionRect());	109 paintRect.unite(m_layoutReplaced.localSelectionRect());

110 paintRect.moveBy(adjustedPaintOffset);	110 paintRect.moveBy(adjustedPaintOffset);

111	111

112 if (!paintInfo.cullRect().intersectsCullRect(paintRect))	112 if (!paintInfo.cullRect().intersectsCullRect(paintRect))

113 return false;	113 return false;

114	114

115 return true;	115 return true;

116 }	116 }

117	117

118 } // namespace blink	118 } // namespace blink

OLD	NEW