540256 - [BC] Border collapse rewrite II

Status: NEW

(Core :: Layout: Tables, defect)

Description

[Bernd]

I hijacked the previous bug 452319

The border collapse code is now refactored, the rules code mess got cleaned up, the code is currently reacting to style changes, the scrolling of row groups got removed, nevertheless the amount of border collapse bugs is huge, too huge in my opinion.

There are now a bunch of reftests in our tree not enough but far more than we had when bug 452319 got filed.

So lets start again with comment 0 from bug 452319, now that a lot of the hacks got removed.
  


> The rewrite will need several preliminary patches and reftests (in CSS test
> suite format, I promiss).
> 
> The first preliminary patch is to change the way we paint backgrounds to have
> it more integrated with the display list system. For the record: the idea is to
> remove the GenericTraversal of the cells, and modify the table painter so that
> it creates display items instead of directly painting. Advantages: we traverse
> the cells only once, and the result is more able to manage optimizations or
> reorders for correctness.
> 
> Then the inevitable newbie questions:
> 
> a) The table painter calls PaintBackgroundWithSC with custom arguments, and the
> generic DisplayBackground item uses PaintBackground, and fetches the arguments
> from the given frame. IIUC, the arguments probably would end up identical, but
> what do we want ? I can create a specific display item wich would store all the
> arguments (kind of a DisplayCustomBackground, which would enable to paint bgs
> not corresponding at all to a real frame), or I can reuse the generic display
> item if we decide that the important information that the table painter has is
> the row/col ownership, and that caching the actual backgrounds/borders is less
> important, or I can do something inbetween.
> 
> Another idea would be to store enough info in the display item for it to go
> retrieve what it needs from the table painter (which we can keep alive until
> the end of painting), but is it worth it ? I'd say that it's interesting at
> least for the offsets instead of calling multiple times ToReferenceFrame...
> 
> b) I think we still want to pass the table's BorderBackground list to the cells
> but I'd like advice (see the comment in nsTableFrame::GenericTraversal)
> 
> Probably more questions to come, but for now I need sleep

Comment 1

[Julien "_FrnchFrgg_" RIVAUD]

I'm not going to work on this bug until mid-march since I'm currently really busy working on my PhD.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Julien, are you going to be able to work on this in the near future?  Or does this need a new owner?

Comment 3

[Bernd]

Just to get the architectural things noted: 

0) One could compute the borders while painting this would make any storage discussion obsolete but could be a performance problem, I am afraid of sluggish scrolling.
1) In order to paint the borders we need for any given colX and rowY (if they count from 0 to numcols where 0 is the left most and numcols the right most border we need the frame who owns this border. A iframe pointer from where one can deduce the real frame would be good enough. 
2) Further we need the owner ship of the corner here we need the owning frame and the direction of the second border that defines the width of the corner or plain the subwidth of the corner. I am not sure that I want the complicated corners outlined in https://wiki.mozilla.org/Gecko:Border_collapse
3) The old system of coalescing identical types of borders as a performance optimization should go away as it impossible the get this correct as soon as dynamic changes or incremental reflows are involved 
4) as the bottom border of one cell is the top of neighboring below cell it would be cool to have a system where double painting is prevented.I am not sure if this is a hard requirement if semitransparent borders are involved. 
5. The border of one cell can be split into multiple pieces as row and colspans might be involved (see for instance http://mxr.mozilla.org/mozilla-central/source/layout/reftests/table-bordercollapse/frame_border_rules_rows.html)

There are multiple design questions:
where and how should the information be stored if at all. The current storage is shown at https://developer.mozilla.org/en/Table_Cellmap_-_Border_Collapse

One question is also how we kill bc table custom painting methods at  nsCSSRendering::DrawTableBorderSegment they are for the dashed and dotted a performance problem.

Although a clever storage might be the opener for easier computation and easier painting.

One qestion is also where this painting should happen dbaron attributed this anther bc bug to the table which is also my gut feeling.

Comment 4

[Julien "_FrnchFrgg_" RIVAUD]

(In reply to Boris Zbarsky (:bz) from comment #2)
> Julien, are you going to be able to work on this in the near future?  Or
> does this need a new owner?

This needs a new owner, I'm completely swamped into my PhD

Comment 5

[Sebastian Zartner [:sebo]]

(In reply to Julien "_FrnchFrgg_" RIVAUD from comment #4)
> (In reply to Boris Zbarsky (:bz) from comment #2)
> > Julien, are you going to be able to work on this in the near future?  Or
> > does this need a new owner?
> 
> This needs a new owner, I'm completely swamped into my PhD

Julien, did you get your PhD? Looks like this bug needs some attention.

Sebastian

Comment 6

[Julien "_FrnchFrgg_" RIVAUD]

I don't work anymore on my PhD, but I'm almost as busy as back then. I won't be able to work on this bug for a while, especially since my knowledge of Gecko' innards severely bitrotted. Next time I can find the time to wrap my mind around that part of the code and try a rewrite is June 2015 so a new owner is probably still in order. Sorry for that.

Comment 7

[Julien "_FrnchFrgg_" RIVAUD]

One note: I probably still have all images of https://wiki.mozilla.org/Gecko:Border_collapse so I might be able to fix that page (to avoid that in the future: is it possible to host images in the wiki ? Or maybe as attachments to a bug but that seems hackish to me). Of course some of the choices in there are overkill for the corner drawing, but at least simple cases could be handled... (And every decision should make it into a reftest.)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> is it possible to host images in the wiki ?

https://wiki.mozilla.org/Special:Upload should do it.

Comment 9

[Sebastian Zartner [:sebo]]

With Stylo coming soon, does this still need to be fixed?

Sebastian

Comment 10

[Boris Zbarsky [:bzbarsky]]

This is layout code, not style code.  Totally unrelated to stylo.