Bug 706859

Summary: After adding a new pdf page, trying to pdf_load_page() the new page returns a wrong page.
Product: MuPDF Reporter: Ardo <aldo.w.buratti>
Component: mupdfAssignee: MuPDF bugs <mupdf-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: robin.watts
Priority: P4    
Version: 1.22.0   
Hardware: All   
OS: All   
Customer: Word Size: ---

Description Ardo 2023-07-09 20:43:56 UTC
Overview
--------
I experienced some unexpected and wrong behaviors when adding pages to a pdf document.

In particular, if a load page K and then try to add a new page at K,
I expect that the previously loaded page is reported as pagenumber K+1,
and the new page as pagenumber K.

Surprisingly, both pages are reported as pagenumber K+1.


Steps to Reproduce
------------------
#include "mupdf/fitz.h"
#include <mupdf/pdf.h>

#include <string.h>
#include <stdio.h>



pdf_page* my_newpage(fz_context* ctx, pdf_document* doc, int pno, float dx, float dy) {
	pdf_obj* obj = NULL;
	pdf_obj* resources = NULL;
	fz_buffer* contents = NULL;
	fz_rect mediabox = { 0, 0, dx, dy };

	fz_var(obj);
	fz_var(resources);
	fz_var(contents);

	fz_try(ctx) {
		// create /Resources and /Contents objects
		resources = pdf_add_new_dict(ctx, doc, 1);
		// ?? contents still NULL ??
		obj = pdf_add_page(ctx, doc, mediabox, 0 /* rotate */, resources, contents);
		pdf_insert_page(ctx, doc, pno, obj); //All existing pages are after the insertion point are shuffled up.
	} fz_always(ctx) {
		if (contents) fz_drop_buffer(ctx, contents);
		if (obj) pdf_drop_obj(ctx, obj);
		if (resources) pdf_drop_obj(ctx, resources);
	} fz_catch(ctx) {
		fprintf(stderr, "Error; %s\n", fz_caught_message(ctx));
		return NULL;
	}

	pdf_page* pagenew = pdf_load_page(ctx, doc, pno);

	return pagenew;
}


int main(int argc, wchar_t* wargv[]) {
	fz_context* ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
	// insert here any valid pdf
	pdf_document* doc = pdf_open_document(ctx, "/tmp/book.pdf");
	fprintf(stderr, "n.of.pages %d\n", pdf_count_pages(ctx, doc));

	pdf_page* p0 = pdf_load_page(ctx, doc, 0);

	fprintf(stderr, "before inserting new page ...\n");
	fprintf(stderr, "pagenum for p0 is %d\n", pdf_lookup_page_number(ctx, doc, p0->obj));
	// ** expected: pagenum for p0 is 0
	//  OK

	int newpno = 0;  //  just before the already loaded page
	pdf_page* pagenew = my_newpage(ctx, doc, newpno, 333, 444);

	fprintf(stderr, "AFTER inserting new page %d...\n", newpno);
	fprintf(stderr, "pagenum for p0 is %d\n", pdf_lookup_page_number(ctx, doc, p0->obj));
	// ** expected: pagenum for p0 is 1
	// ** OK
	fprintf(stderr, "pagenum for pnew is %d\n", pdf_lookup_page_number(ctx, doc, pagenew->obj));
	// ** expected: pagenum for pnew is 0
	// KO: result is: pagenum for pnew is 1
	//  what is worst is that  .. pnew points to the old page-0  (now shifted as page-1)


	pdf_drop_document(ctx, doc);
	return 0;
}

Actual Results: 
---------------
....
AFTER inserting new page 0...
pagenum for p0 is 1
pagenum for pnew is 1


Expected Results:
-----------------
...
AFTER inserting new page 0...
pagenum for p0 is 1
pagenum for pnew is 0

Build Date & Hardware:
----------------------
mupdf-1.22.2

Additional Information:
-----------------------
This only happens when I try to insert a new page K and the page K is already loaded.
If other pages (<>K) are loaded, they are correctly renumbered, and the new page K is correctly loaded.

I suspect this is due because the currently opened pages (fz_page) have a member named 'number' that is NOT 'renumbered'
when a page is added/deleted; this 'number' is used in pdf_load_page() when looking for already opened pages ..
Comment 1 Robin Watts 2023-07-17 16:47:16 UTC
Fixed with:

commit 1ae74cec7bfebc54d900fb084a5ab4e604b2d368 (golden/master)
Author: Robin Watts <Robin.Watts@artifex.com>
Date:   Mon Jul 17 17:14:16 2023 +0100

    Bug 706859: Keep fz layer up to date with changes in pdf pages.

    When we pdf_insert_page or pdf_delete_page, we modify the pdf
    page tree, but don't adjust the list of opened pages that the
    fz layer is holding.

    Make those modifications here.

Thanks for the report!