Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

Table page-break improvements #11291

Closed
wants to merge 1 commit into from
Closed

Table page-break improvements #11291

wants to merge 1 commit into from

Conversation

skotchio
Copy link

@skotchio skotchio commented May 6, 2013

Bump tables to the next page if we cannot fit the caption, thead
and first cell on the current page. Ideally this should be checking
the height of all cells in the first row in the same manner as is
done in the table section render but as this is more of a nice to
have I've done the minimal amount of work for now.

We now only layout section rows once each section has been positioned
within the table, and only supply head and foot reservation heights
to the appropriate sections.

When re-drawing the thead and tfoot only do so if the rect we are
painting is explicitly before/after the "original" thead/tfoot
rather than simply outside of its boundaries. This prevents issues
with re-drawing a redundant head when bumping tables to the next page.

Re-work the table section row layout code so that it's a bit more
streamlined. Additionally, to cater for cells spanning multiple rows
we avoid page breaks in the middle of their content. Note that rows
are always bumped to the next page even if their content doesn't
fit on a full page so as to avoid daft situtations like having the
first 1px on the preceeding page.

Bump tables to the next page if we cannot fit the caption, thead
and first cell on the current page. Ideally this should be checking
the height of all cells in the first row in the same manner as is
done in the table section render but as this is more of a nice to
have I've done the minimal amount of work for now.

We now only layout section rows once each section has been positioned
within the table, and only supply head and foot reservation heights
to the appropriate sections.

When re-drawing the thead and tfoot only do so if the rect we are
painting is explicitly before/after the "original" thead/tfoot
rather than simply outside of its boundaries. This prevents issues
with re-drawing a redundant head when bumping tables to the next page.

Re-work the table section row layout code so that it's a bit more
streamlined. Additionally, to cater for cells spanning multiple rows
we avoid page breaks in the middle of their content. Note that rows
are always bumped to the next page even if their content doesn't
fit on a full page so as to avoid daft situtations like having the
first 1px on the preceeding page.
@ariya
Copy link
Owner

ariya commented May 6, 2013

Any issue associated with this one? If not, please create it first.

@trvrnrth
Copy link
Contributor

trvrnrth commented May 8, 2013

Just to chime in here this looks to have been taken from my branch of qt at https://gitorious.org/~trvrnrth/qt/trvrnrths-qt which I put together to port across repeating table headers and footers to wkhtmltopdf. There is some discussion at https://code.google.com/p/wkhtmltopdf/issues/detail?id=168.

This particular commit resolved a few issues I discovered along the way, more or less as described in the commit message. I'm afraid I don't really have any further details to hand, though what I will say is that there are also a few later commits in my repo that would want pulling in too.

@ariya
Copy link
Owner

ariya commented May 14, 2013

Has this been pushed to upstream QtWebKit? That'll be the preferred solution.

@ariya
Copy link
Owner

ariya commented May 22, 2013

@vovan22 see my question above?

@ariya
Copy link
Owner

ariya commented May 24, 2013

@vovan22 On the contrary, moving to a fresher Qt is the plan, see #10448.

It's in everyone's best interest to have any enhancement upstreamed so that we don't carry the burden of maintenance.

@trvrnrth
Copy link
Contributor

I'd agree that getting the changes upstream would be best for all concerned. Was any attempt ever made to do so for the changes made for issue #10880 / #211, specifically commits 5c87852 and 2d778f6 which the changes in my gitorious branch build upon?

@ariya
Copy link
Owner

ariya commented May 26, 2013

@vovan22 Can you clarify your question?

@JamesMGreene
Copy link
Collaborator

@ariya I think @vovan22's question is "Why can't you just drop in Qt 5 now?".

@ariya
Copy link
Owner

ariya commented May 27, 2013

@vovan22 There's no known problem yet. It's just a lot of work.

@vitallium
Copy link
Collaborator

@vovan22, we haven't set a date. We'll switch to Qt5 as soon as we're ready.

@ariya
Copy link
Owner

ariya commented May 30, 2013

It's better than anything not directly related to this issue is discussed in the mailing-list or in that Qt 5 update issue.

@ariya
Copy link
Owner

ariya commented Jun 9, 2013

In the mean time, I'll put this on patches/ directory.

ariya added a commit that referenced this pull request Jun 9, 2013
From d78182d3a6451522f239ee1ecbb71863eb053792 Mon Sep 17 00:00:00 2001
From: Artem Baranovskiy <likejavascript@gmail.com>
Date: Mon, 6 May 2013 08:01:01 +0400
Subject: [PATCH] Table page-break improvements

See issue #11291 #11291
@ariya ariya closed this Jun 9, 2013
@trvrnrth
Copy link
Contributor

As I mentioned earlier, the patch provided here was taken from my branch of qt on gitorious. That's absolutely fine, and it's great to see it being included. It would however be best to base it on the latest head revision as even when this pull request was opened the patch provided was out of date and the later commits resolve some further edge cases (including an issue trying to measure a non-existent cell which causes a segfault). I could open a new pull request with an updated patch if you'd prefer?

@trvrnrth
Copy link
Contributor

I've opened #11490

ariya pushed a commit that referenced this pull request Jan 11, 2014
Includes the following fixes taken from trvrnrths-qt:
- Ensure we have a first cell to measure when checking required table
  height
- Handle page break edge case with exactly fitting last table row
  In the case where the last table row on a page fitted exactly no
  extra offset was afforded to the next row. This resulted in no
  space being left for the painting of the table header on the next
  page.
- Fix segfault when checking heights for pagination if table body does
  not have a cell at 0,0

See #11291 and
#11490.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants