When PaintBCBorders uses the damage rect to find the row and column to start painting at, we get into trouble with painting looking different depending on what the damage rect is. I think this is partly to do with spanning cells, but I haven't fully debugged it. Always starting damageArea at the top left of the table fixes the bug. IMHO this optimization isn't worth it. I'll attach a patch to turn it off. If you think the optimization is worth it, then feel free to fix the underlying problem :-). The problem shows up in several testcases, e.g. layout/html/tests/table/marvin/backgr_position-table.html
Created attachment 176498 [details] [diff] [review] fix This is a simple fix, the only question is whether it's really OK to just turn off this optimization. Note that we still get the benefit of stopping painting when the rows or columns go beyond the damage rect, so painting the top of a huge table is still no problem. I'm also correcting the erasure of verInfo to erase the whole allocated array. Actually I suspect, looking at the code, that verInfo could just be length damageRect.width and the last element is not currently used, but I'm not 100% sure.
14 years ago
Comment on attachment 176498 [details] [diff] [review] fix sr=bzbarsky if bernd oks this.
Attachment #176498 - Flags: superreview?(bzbarsky) → superreview+
This will make scrolling slower. As we will not only paint a small strip but the whole table if I understand it correctly. There is pretty much effort in the code to get the damage area to avoid just this.
> IMHO this optimization isn't worth it. Why do we have a paint damage area then at all in our code? The correct patch, instead of hacking around without proper commenting why this is necessary, would be to paint the table cell borders by the table cells. If you feel that you need this for your testing, feel free to fix this, this would really help. I will not have time for this, as I will be busy on a foreseeable future with crash fixes.
I mean I can help you with this but are not able to fix this by myself in a foreseeable future. I am somehow reluctant to pass hacks into the code just to make a test/compiler happy. If you think that you *really* need this hack instead of a fix then please comment: a) that it is a hack b) why we need this hack c) what the proper fix would require
Comment on attachment 176498 [details] [diff] [review] fix alright, I don't really need this.
Attachment #176498 - Flags: review?(bernd_mozilla) → review-
14 years ago
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.