Load most annotations, even if they are missing appearances.

Filter out Link and Popup annotations.

Links are not comments, popup annotations are auxiliary information
for other annotations, so neither of these types should be present
in our list of annotations, but all other annotations should be there,
whether they have appearance streams or not.

Ensure has_new_ap is zero when first loaded, and changed if either
the active AP object is changed or the current AP content stream
is updated.

9 files changed, 59 insertions, 150 deletions

diff --git a/include/mupdf/pdf/annot.h b/include/mupdf/pdf/annot.h
index 58c11a33..94a8f33f 100644
--- a/include/mupdf/pdf/annot.h
+++ b/include/mupdf/pdf/annot.h
@@ -258,8 +258,9 @@ void pdf_set_free_text_details(fz_context *ctx, pdf_annot *annot, fz_point *pos,
 /*
 	pdf_new_annot: Internal function for creating a new pdf annotation.
 */
-pdf_annot *pdf_new_annot(fz_context *ctx, pdf_page *page);
+pdf_annot *pdf_new_annot(fz_context *ctx, pdf_page *page, pdf_obj *obj);
 
+void pdf_update_appearance(fz_context *ctx, pdf_annot *annot);
 void pdf_dirty_annot(fz_context *ctx, pdf_annot *annot);
 
 #endif
diff --git a/include/mupdf/pdf/document.h b/include/mupdf/pdf/document.h
index 5b5963ba..5ab0fa2c 100644
--- a/include/mupdf/pdf/document.h
+++ b/include/mupdf/pdf/document.h
@@ -695,8 +695,6 @@ struct pdf_document_s
 	int recalculating;
 	int dirty;
 
-	void (*update_appearance)(fz_context *ctx, pdf_document *doc, pdf_annot *annot);
-
 	pdf_doc_event_cb *event_cb;
 	void *event_cb_data;
 
diff --git a/include/mupdf/pdf/resource.h b/include/mupdf/pdf/resource.h
index 05de95b2..c98ba9df 100644
--- a/include/mupdf/pdf/resource.h
+++ b/include/mupdf/pdf/resource.h
@@ -82,8 +82,6 @@ void pdf_drop_pattern(fz_context *ctx, pdf_pattern *pat);
 pdf_obj *pdf_new_xobject(fz_context *ctx, pdf_document *doc, const fz_rect *bbox, const fz_matrix *mat);
 void pdf_update_xobject_contents(fz_context *ctx, pdf_document *doc, pdf_obj *form, fz_buffer *buffer);
 
-void pdf_update_appearance(fz_context *ctx, pdf_document *doc, pdf_annot *annot);
-
 pdf_obj *pdf_xobject_resources(fz_context *ctx, pdf_obj *xobj);
 fz_rect *pdf_xobject_bbox(fz_context *ctx, pdf_obj *xobj, fz_rect *bbox);
 fz_matrix *pdf_xobject_matrix(fz_context *ctx, pdf_obj *xobj, fz_matrix *matrix);
diff --git a/platform/java/mupdf_native.c b/platform/java/mupdf_native.c
index 754d31a2..65ba80e5 100644
--- a/platform/java/mupdf_native.c
+++ b/platform/java/mupdf_native.c
@@ -9104,7 +9104,7 @@ FUN(PDFAnnotation_updateAppearance)(JNIEnv *env, jobject self)
 	fz_try(ctx)
 	{
 		pdf_dict_del(ctx, annot->obj, PDF_NAME_AP); /* nuke old AP */
-		pdf_update_appearance(ctx, annot->page->doc, annot);
+		pdf_update_appearance(ctx, annot);
 		pdf_update_annot(ctx, annot); /* ensure new AP is put into annot */
 	}
 	fz_catch(ctx)
diff --git a/source/pdf/pdf-annot-edit.c b/source/pdf/pdf-annot-edit.c
index 4f5ffadd..7708f9d7 100644
--- a/source/pdf/pdf-annot-edit.c
+++ b/source/pdf/pdf-annot-edit.c
@@ -129,9 +129,6 @@ pdf_create_annot(fz_context *ctx, pdf_page *page, enum pdf_annot_type type)
 		/* Make printable as default */
 		pdf_dict_put_int(ctx, annot_obj, PDF_NAME_F, PDF_ANNOT_IS_PRINT);
 
-		annot = pdf_new_annot(ctx, page);
-		annot->ap = NULL;
-
 		/*
 			Both annotation object and annotation structure are now created.
 			Insert the object in the hierarchy and the structure in the
@@ -141,7 +138,9 @@ pdf_create_annot(fz_context *ctx, pdf_page *page, enum pdf_annot_type type)
 		pdf_update_object(ctx, doc, ind_obj_num, annot_obj);
 		ind_obj = pdf_new_indirect(ctx, doc, ind_obj_num, 0);
 		pdf_array_push(ctx, annot_arr, ind_obj);
-		annot->obj = pdf_keep_obj(ctx, ind_obj);
+
+		annot = pdf_new_annot(ctx, page, ind_obj);
+		annot->ap = NULL;
 
 		/*
 			Linking must be done after any call that might throw because
@@ -1099,20 +1098,6 @@ pdf_add_annot_ink_list(fz_context *ctx, pdf_annot *annot, int n, fz_point p[])
 	pdf_dirty_annot(ctx, annot);
 }
 
-static void find_free_font_name(fz_context *ctx, pdf_obj *fdict, char *buf, int buf_size)
-{
-	int i;
-
-	/* Find a number X such that /FX doesn't occur as a key in fdict */
-	for (i = 0; 1; i++)
-	{
-		fz_snprintf(buf, buf_size, "F%d", i);
-
-		if (!pdf_dict_gets(ctx, fdict, buf))
-			break;
-	}
-}
-
 void
 pdf_set_text_annot_position(fz_context *ctx, pdf_annot *annot, fz_point pt)
 {
@@ -1299,6 +1284,20 @@ pdf_set_annot_author(fz_context *ctx, pdf_annot *annot, const char *author)
 	pdf_dirty_annot(ctx, annot);
 }
 
+static void find_free_font_name(fz_context *ctx, pdf_obj *fdict, char *buf, int buf_size)
+{
+	int i;
+
+	/* Find a number X such that /FX doesn't occur as a key in fdict */
+	for (i = 0; 1; i++)
+	{
+		fz_snprintf(buf, buf_size, "F%d", i);
+
+		if (!pdf_dict_gets(ctx, fdict, buf))
+			break;
+	}
+}
+
 void
 pdf_set_free_text_details(fz_context *ctx, pdf_annot *annot, fz_point *pos, char *text, char *font_name, float font_size, float color[3])
 {
diff --git a/source/pdf/pdf-annot.c b/source/pdf/pdf-annot.c
index 3325c828..56b42e48 100644
--- a/source/pdf/pdf-annot.c
+++ b/source/pdf/pdf-annot.c
@@ -400,16 +400,19 @@ pdf_annot_transform(fz_context *ctx, pdf_annot *annot, fz_matrix *annot_ctm)
 	fz_pre_scale(fz_translate(annot_ctm, x, y), w, h);
 }
 
-pdf_annot *pdf_new_annot(fz_context *ctx, pdf_page *page)
+pdf_annot *pdf_new_annot(fz_context *ctx, pdf_page *page, pdf_obj *obj)
 {
-	pdf_annot *annot = fz_new_derived_annot(ctx, pdf_annot);
+	pdf_annot *annot;
+
+	annot = fz_new_derived_annot(ctx, pdf_annot);
 
 	annot->super.drop_annot = (fz_annot_drop_fn*)pdf_drop_annot_imp;
 	annot->super.bound_annot = (fz_annot_bound_fn*)pdf_bound_annot;
 	annot->super.run_annot = (fz_annot_run_fn*)pdf_run_annot;
 	annot->super.next_annot = (fz_annot_next_fn*)pdf_next_annot;
 
-	annot->page = page;
+	annot->page = page; /* only borrowed, as the page owns the annot */
+	annot->obj = pdf_keep_obj(ctx, obj);
 
 	return annot;
 }
@@ -417,121 +420,35 @@ pdf_annot *pdf_new_annot(fz_context *ctx, pdf_page *page)
 void
 pdf_load_annots(fz_context *ctx, pdf_page *page, pdf_obj *annots)
 {
-	pdf_document *doc = page->doc;
-	pdf_annot *annot, **itr;
-	pdf_obj *obj, *ap, *as, *n;
-	int i, len, keep_annot;
-
-	fz_var(annot);
-	fz_var(itr);
-	fz_var(keep_annot);
-
-	itr = &page->annots;
-
-	len = pdf_array_len(ctx, annots);
-	/*
-	Create an initial linked list of pdf_annot structures with only the obj field
-	filled in. We do this because update_appearance has the potential to change
-	the annot array, so we don't want to be iterating through the array while
-	that happens.
-	*/
-	fz_try(ctx)
-	{
-		for (i = 0; i < len; i++)
-		{
-			obj = pdf_array_get(ctx, annots, i);
-
-			annot = pdf_new_annot(ctx, page);
-			*itr = annot;
-			annot->obj = pdf_keep_obj(ctx, obj);
-			itr = &annot->next;
-		}
-	}
-	fz_catch(ctx)
-	{
-		pdf_drop_annots(ctx, page->annots);
-		page->annots = NULL;
-		fz_rethrow(ctx);
-	}
+	pdf_annot *annot;
+	pdf_obj *subtype;
+	int i, n;
 
-	/*
-	Iterate through the newly created annot linked list, using a double pointer to
-	facilitate deleting broken annotations.
-	*/
-	itr = &page->annots;
-	while (*itr)
+	n = pdf_array_len(ctx, annots);
+	for (i = 0; i < n; ++i)
 	{
-		annot = *itr;
-
-		fz_try(ctx)
+		pdf_obj *obj = pdf_array_get(ctx, annots, i);
+		if (obj)
 		{
-			pdf_hotspot *hp = &doc->hotspot;
-
-			n = NULL;
-
-			if (doc->update_appearance)
-				doc->update_appearance(ctx, doc, annot);
-
-			obj = annot->obj;
-			ap = pdf_dict_get(ctx, obj, PDF_NAME_AP);
-			as = pdf_dict_get(ctx, obj, PDF_NAME_AS);
-
-			/* We only collect annotations with an appearance
-			 * stream into this list, so remove any that don't
-			 * (such as links) and continue. */
-			keep_annot = pdf_is_dict(ctx, ap);
-			if (!keep_annot)
-				break;
-
-			if (hp->num == pdf_to_num(ctx, obj) && (hp->state & HOTSPOT_POINTER_DOWN))
+			subtype = pdf_dict_get(ctx, obj, PDF_NAME_Subtype);
+			if (pdf_name_eq(ctx, subtype, PDF_NAME_Link))
+				continue;
+			if (pdf_name_eq(ctx, subtype, PDF_NAME_Popup))
+				continue;
+
+			annot = pdf_new_annot(ctx, page, obj);
+			fz_try(ctx)
 			{
-				n = pdf_dict_get(ctx, ap, PDF_NAME_D); /* down state */
+				pdf_update_annot(ctx, annot);
+				annot->has_new_ap = 0;
 			}
+			fz_catch(ctx)
+				fz_warn(ctx, "could not update appearance for annotation");
 
-			if (n == NULL)
-				n = pdf_dict_get(ctx, ap, PDF_NAME_N); /* normal state */
-
-			/* lookup current state in sub-dictionary */
-			if (!pdf_is_stream(ctx, n))
-				n = pdf_dict_get(ctx, n, as);
-
-			annot->ap = NULL;
-			annot->has_new_ap = 1;
-
-			if (pdf_is_stream(ctx, n))
-			{
-				annot->ap = pdf_keep_obj(ctx, n);
-			}
-			else
-				fz_warn(ctx, "no appearance stream for annotation %d 0 R", pdf_to_num(ctx, annot->obj));
-
-			if (obj == doc->focus_obj)
-				doc->focus = annot;
-
-			/* Move to next item in the linked list */
-			itr = &annot->next;
-		}
-		fz_catch(ctx)
-		{
-			if (fz_caught(ctx) == FZ_ERROR_TRYLATER)
-			{
-				pdf_drop_annots(ctx, page->annots);
-				page->annots = NULL;
-				fz_rethrow(ctx);
-			}
-			keep_annot = 0;
-			fz_warn(ctx, "ignoring broken annotation");
-		}
-		if (!keep_annot)
-		{
-			/* Move to next item in the linked list, dropping this one */
-			*itr = annot->next;
-			annot->next = NULL; /* Required because pdf_drop_annots follows the "next" chain */
-			pdf_drop_annots(ctx, annot);
+			*page->annot_tailp = annot;
+			page->annot_tailp = &annot->next;
 		}
 	}
-
-	page->annot_tailp = itr;
 }
 
 pdf_annot *
diff --git a/source/pdf/pdf-appearance.c b/source/pdf/pdf-appearance.c
index 1c0189c7..22621fe5 100644
--- a/source/pdf/pdf-appearance.c
+++ b/source/pdf/pdf-appearance.c
@@ -2451,12 +2451,13 @@ void pdf_set_signature_appearance(fz_context *ctx, pdf_document *doc, pdf_annot
 	}
 }
 
-void pdf_update_appearance(fz_context *ctx, pdf_document *doc, pdf_annot *annot)
+void pdf_update_appearance(fz_context *ctx, pdf_annot *annot)
 {
+	pdf_document *doc = annot->page->doc;
 	pdf_obj *obj = annot->obj;
 	pdf_obj *ap = pdf_dict_get(ctx, obj, PDF_NAME_AP);
 
-	if (!ap || !pdf_dict_get(ctx, ap, PDF_NAME_N) || pdf_obj_is_dirty(ctx, obj))
+	if (!ap || !pdf_dict_get(ctx, ap, PDF_NAME_N) || pdf_obj_is_dirty(ctx, obj) || annot->needs_new_ap)
 	{
 		enum pdf_annot_type type = pdf_annot_type(ctx, annot);
 		switch (type)
@@ -2508,6 +2509,7 @@ void pdf_update_appearance(fz_context *ctx, pdf_document *doc, pdf_annot *annot)
 				pdf_update_combobox_appearance(ctx, doc, obj);
 				break;
 			}
+			annot->has_new_ap = 1;
 			break;
 		case PDF_ANNOT_TEXT:
 			pdf_update_text_annot_appearance(ctx, doc, annot);
@@ -2537,8 +2539,7 @@ pdf_update_annot(fz_context *ctx, pdf_annot *annot)
 	pdf_document *doc = annot->page->doc;
 	pdf_obj *obj, *ap, *as, *n;
 
-	if (doc->update_appearance)
-		doc->update_appearance(ctx, doc, annot);
+	pdf_update_appearance(ctx, annot);
 
 	obj = annot->obj;
 
@@ -2550,12 +2551,8 @@ pdf_update_annot(fz_context *ctx, pdf_annot *annot)
 		pdf_hotspot *hp = &doc->hotspot;
 
 		n = NULL;
-
 		if (hp->num == pdf_to_num(ctx, obj) && (hp->state & HOTSPOT_POINTER_DOWN))
-		{
 			n = pdf_dict_get(ctx, ap, PDF_NAME_D); /* down state */
-		}
-
 		if (n == NULL)
 			n = pdf_dict_get(ctx, ap, PDF_NAME_N); /* normal state */
 
@@ -2563,13 +2560,13 @@ pdf_update_annot(fz_context *ctx, pdf_annot *annot)
 		if (!pdf_is_stream(ctx, n))
 			n = pdf_dict_get(ctx, n, as);
 
-		pdf_drop_obj(ctx, annot->ap);
-		annot->ap = NULL;
-
-		if (pdf_is_stream(ctx, n))
+		if (annot->ap != n)
 		{
-			annot->ap = pdf_keep_obj(ctx, n);
+			pdf_drop_obj(ctx, annot->ap);
+			annot->ap = NULL;
+			if (pdf_is_stream(ctx, n))
+				annot->ap = pdf_keep_obj(ctx, n);
+			annot->has_new_ap = 1;
 		}
-		annot->has_new_ap = 1;
 	}
 }
diff --git a/source/pdf/pdf-xref.c b/source/pdf/pdf-xref.c
index 013c6b77..72e0dc43 100644
--- a/source/pdf/pdf-xref.c
+++ b/source/pdf/pdf-xref.c
@@ -2248,7 +2248,6 @@ pdf_new_document(fz_context *ctx, fz_stream *file)
 	doc->super.count_pages = (fz_document_count_pages_fn*)pdf_count_pages;
 	doc->super.load_page = (fz_document_load_page_fn*)pdf_load_page;
 	doc->super.lookup_metadata = (fz_document_lookup_metadata_fn*)pdf_lookup_metadata;
-	doc->update_appearance = pdf_update_appearance;
 
 	pdf_lexbuf_init(ctx, &doc->lexbuf.base, PDF_LEXBUF_LARGE);
 	doc->file = fz_keep_stream(ctx, file);
diff --git a/source/tools/murun.c b/source/tools/murun.c
index 8b9cd29d..03e55606 100644
--- a/source/tools/murun.c
+++ b/source/tools/murun.c
@@ -4430,7 +4430,7 @@ static void ffi_PDFAnnotation_updateAppearance(js_State *J)
 	fz_context *ctx = js_getcontext(J);
 	pdf_annot *annot = js_touserdata(J, 0, "pdf_annot");
 	fz_try(ctx)
-		pdf_update_appearance(ctx, annot->page->doc, annot);
+		pdf_update_appearance(ctx, annot);
 	fz_catch(ctx)
 		rethrow(J);
 }