View Issue Details

IDProjectCategoryView StatusLast Update
0012292ScribusCanvaspublic2019-03-07 13:06
Reporterale Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status newResolutionopen 
Product Version1.5.0 
Target VersionFixed in Version 
Summary0012292: PageItem::getPages(): Provide a standard way to know on which page(s) an item is
DescriptionEach PageItem has an OwnPage member which mostly defines on which page the item is.
But -- according to Jean (and my experience) -- it's only reliable if you want to make sense of the coordinates of the item itself.

This leads to several methods spread across the code base implementing the page detection. I've found ScribusDoc::fixItemPageOwner, ScribusDoc::OnPage, PDFLibCore::PDF_ProcessItem, and I am implementing my own in the Epub exporter.

I suggest that each item has a member telling on which pages it is (PageItem::getPages()?) and that its OwnPage value is set to something meaningful.

Since it can be expensive to keep this data correct while moving an object (or doing other repetitive tasks) I suggest that only a "moved" flag is set as soon as the relation between the item and the page(s) might be changed.

Methods needing to know the item's page, would then simply request it from the item trough PageItem::getPages(). This method would check if the item has been moved, only then recalculate the value if necessary and return the value.

The return value would be a list of pages and a further member could eventually also store which page contains the biggest part of the item.

I can provide a PageItem::getPages() if you think this is a useful and can be integrated in 1.5svn ASAP, but I would first need to know where the page relationship is needed (on top of the three mentioned above) and if there are conditions + needs I have could have ignored.
Additional InformationThis is the method I had drafted for the Epub plugin. It's not finished, yet... it's just here to show the direction I'm taking and to start the discussion.

"""
/**
 * Returns a list of ScPage where the item appears. If the item is fully ion the scratch space,
 * an empty vector is returned.
 * TODO:
 * - This (or a similar) method should replace the (very) similar calculations in
 * ScribusDoc::fixItemPageOwner, ScribusDoc::OnPage and PDFLibCore::PDF_ProcessItem
 * It should go to PageItem (or ScPage) and it should be cached in memory + eventually in the .SLA
 * --> PageItem::getPages()
 * (According to jghali OwnPage should only be used make sense of the coordinates of an item,
 * which are stored in relation to its own page)
 */
QList<ScPage *> EpubExportScribusDoc::getPagesWithItem(PageItem* item)
{
    QList<ScPage *> result;

    // some woodoo adjustings
    if (item->isGroup())
        item->asGroupFrame()->adjustXYPosition();
    item->setRedrawBounding();

    double itemLineWidth = item->lineWidth();
    QRect pageRect;
    QRect itemRect = QRect(
        static_cast<int>(item->BoundingX - itemLineWidth / 2.0), // x
        static_cast<int>(item->BoundingY - itemLineWidth / 2.0), // y
        static_cast<int>(qMax(item->BoundingW + itemLineWidth, 1.0)), // w
        static_cast<int>(qMax(item->BoundingH + itemLineWidth, 1.0)) // h
    );

    bool fullyOnOwnPage = false;
    // First check if the element is fully on its OwnPage
    // OwnPage is an indicator of where the item could be, but it's not reliable.
    if (item->OwnPage > -1)
    {
        ScPage* page = docCurrent->DocPages.at(item->OwnPage); // TODO: use the real page that we are handling
        if (getPageRect(page).contains(itemRect)) {
            result.append(page);
            fullyOnOwnPage = true;
        }
    }

    // If the item is not fully on the OwnPage, check on all pages (in the range)
    if (!fullyOnOwnPage)
    {
        // TODO: if creating the QRect is expensive, we can create a list of pages' QRects
        // before cycling through the items
        bool allPages = pageRange.isEmpty();
        int n = allPages ? docCurrent->DocPages.count() : pageRange.count();
        for (int i = 0; i < n; ++i)
        {
            ScPage* page = docCurrent->DocPages.at(allPages ? i : pageRange.at(i) - 1);
            if (getPageRect(page).intersects(itemRect))
                result.append(page);
            // TODO: we can use rect.intersected() to get a rectangle and calculate the area of the page
            // that has the biggest intersection and use it as the "main page";
            // or we can use the first page where the intersection occurs (two different uses)
        }
        // TODO: if OwnPage == -1 and a page has been found, fix OwnPage in the item
    }

    return result;
}
"""
Tagsdiscussion
PatchYes

Activities

JLuc

2019-03-01 17:53

developer   ~0045947

Such a feature would be usefull

aiena

2019-03-07 12:37

reporter   ~0045976

We need to decide on a uniform methodology to handle OwnPage = "-1". I think we should ask the user to resolve these manually like how preflight does.

ale

2019-03-07 13:06

manager   ~0045977

OwnPage = -1 should probably not be possible.

what we need are clear rules for setting OwnPage.

in my experience, copy pasting does it always correctly,

problems arise with

- items moved from a page to a different one
- items that are on the scratch space.

one important step would probably be, to make OwnPage a private class variable and provide a function to access it (and being in charge of keeping the value up to date).
and how are items handled which are in the scratch page?

Issue History

Date Modified Username Field Change
2014-05-01 07:07 ale New Issue
2016-05-16 00:23 Kunda Patch => Yes
2016-05-16 00:23 Kunda Tag Attached: discussion
2019-03-01 17:53 JLuc Note Added: 0045947
2019-03-07 12:37 aiena Note Added: 0045976
2019-03-07 13:06 ale Note Added: 0045977