Summary: | Rename the "p" member variable of the PaintInfo struct to "context" | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mitz | ||||||
Priority: | P2 | ||||||||
Version: | 420+ | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Attachments: |
|
Description
Sam Weinig
2006-10-31 15:23:43 PST
Created attachment 11314 [details]
patch
Patch renames the 'p' member to 'context' and the 'r' member to 'rect'. No changelog has been to added yet as while I was making the patch several changes were made to near by rendering code that I will have to account for. I'll get an updated patch up asap.
Comment on attachment 11314 [details]
patch
Wow! Very few comments:
This can be replaced with 'RenderStyle* style = m_object->style(m_firstLine)':
+ RenderStyle* style = m_firstLine ? m_object->firstLineStyle() : m_object->style();
This is not equivalent and probably wrong:
- if (inlineFlow)
- ASSERT(m_layer); // The only way a compact/run-in/inline could paint like this is if it has a layer.
+ ASSERT(inlineFlow && m_layer); // The only way a compact/run-in/inline could paint like this is if it has a layer.
(the above change appears twice in the patch)
You didn't change this, but I don't understand why you need to round trip through RGBA:
+ context->setFillColor(color.rgb());
+ context->setFillColor(color.rgb());
This multi-line for loop could use braces:
for (RenderObject* child = firstChild(); child; child = child->nextSibling())
if (child->isTableSection())
- child->paint(paintInfo, _tx, _ty);
+ child->paint(info, tx, ty);
Missing space before the =:
+ mh= max(0, h - (paintInfo.rect.y() - ty));
For consistency (within this patch, at least) the || should go on the end of the first line:
- if (i.phase == PaintPhaseCollapsedTableBorders && style()->visibility() == VISIBLE) {
- if (_ty - table()->outerBorderTop() >= i.r.bottom() + os || _ty + _topExtra + m_height + _bottomExtra + table()->outerBorderBottom() <= i.r.y() - os)
+ if (paintInfo.phase == PaintPhaseCollapsedTableBorders && style()->visibility() == VISIBLE) {
+ if (ty - table()->outerBorderTop() >= paintInfo.rect.bottom() + os
+ || ty + _topExtra + m_height + _bottomExtra + table()->outerBorderBottom() <= paintInfo.rect.y() - os)
I think the Pen() is redundant here:
+ paintInfo.context->setPen(Pen(o->style()->color()));
+ paintInfo.context->setPen(Pen(leftSeparatorColor));
+ paintInfo.context->setPen(Pen(rightSeparatorColor));
No need for the last semicolon:
+ virtual bool isWidget() const { return true; };
Created attachment 11327 [details]
updated patch
The updated patch addresses Mitz's concerns, is updated for the current ToT, and adds a ChangeLog.
Comment on attachment 11327 [details]
updated patch
r=me
|