Log of #mupdf at irc.freenode.net.

Search:
 <<<Back 1 day (to 2020/12/14)Fwd 1 day (to 2020/12/16)>>>20201215 
artifexirc-bot <Robin_Watts> SpeakerToMeat: I'm really not an iOS guy, but I thought you could use iTunes to copy files 'into' the MuPDF sandbox?01:01.12 
SpeakerToMeat I'll try that once I compile it01:01.45 
  There's no prebuilt around for such an old ios it seems01:01.55 
sebras @Robin_Watts have we removed our old mupdf built for ios from the appstore?01:12.49 
  seems so, ok then I'm in no position to help SpeakerToMeat, but Robin_Watts' suggestion sounded reasonable.01:14.41 
  @Robin_Watts I'm still unsure about has_new_ap and needs_new_ap, I'm inclined to LGTM the commit, but I need to remember to ask ator about this in january.02:14.47 
  but there's one thing in pdf_annot_ap() I don't get in the updated variant. why does the normal case and button case differ if they are not pushbuttons? it looks to me like these two cases could be merged?02:18.40 
  Re: "Renumber page and annotation objects as part of saving."02:19.12 
  would we need to also iterate over all widgets in rnumber_pages()?02:19.37 
  you are also adding pdf_page->next and pdf_page->prev, but I can't see that you've introduce usage for them?02:20.22 
  s/for/of/02:20.27 
  LGTM "Add fz_stdods debugging output stream for windows."02:23.52 
  Re: "Add pdf_dump_obj debugging function."02:25.02 
  it seems to me that pdf_dump_obj() is similar enough to pdf_debug_obj().02:25.48 
  maybe there should be a global fz_output * for debugging that is used by pdf_debug_obj() and friends and then the global fz_output is initialized differently depending on whether it is win32 or not?02:27.13 
  or is there something special you require of pdf_dump_obj() for it to be useful?02:27.54 
  Re: "Add journalling API to MuPDF" the message is missing a trailing period.02:28.56 
  why is journaling not always used?02:29.19 
  we rarely label things as public API. is it smart to do it in this case given that this is the first implementation? I'm thinking that we might want to wait a bit with that until we see that we don't end up strange situations that require changing something.02:37.03 
  (btw, if journalling is always enabled we don't need to add pdf_enable_journal() to the JS and JNI APIs)02:38.10 
  if we can only run pdf_dict_put() within an operation, then doesn't the call to pdf_dict_put_string() in pdf_write_digest() which is called from complete_signatures() which happens when saving a newly signed PDFs cause a problem?02:42.39 
  this put is used to update the object in memory with the accurate digest value after it has been computed.02:43.10 
  I guess we ought to be updating /ByteRange there too, but for some reason we aren't.02:43.54 
  maybe this should be an implicit operation? but it happens _after_ saving.02:44.12 
  no, an normal operaiton called "Computing digest" perhaps.02:44.52 
  Computing Signature Digests02:45.05 
  on a high level it seems reasonable so far, I'll review the details of the pdf_journal tomorrow.02:45.48 
artifexirc-bot <Robin_Watts> @sebras Morning. Are you awake?12:01.41 
  <Robin_Watts> I've lost track of what virtual timezone you are inhabiting currently 🙂12:02.15 
sebras @Robin_Watts so have I. :)12:12.00 
  @Robin_Watts awake.12:12.05 
artifexirc-bot <Robin_Watts> Thanks for the reviews so far.12:12.28 
  <Robin_Watts> I've just put up a shading change.12:12.43 
  <Robin_Watts> It fixes a bug, without making any changes in the cluster changes (just doing a final clusterpush to check that).12:13.12 
  <Robin_Watts> It fixes a bug, without making any changes in the cluster tests (just doing a final clusterpush to check that).12:13.28 
  <Robin_Watts> I'll test a simplified version of pdf_annot_ap after that in line with your comments.12:14.00 
  <Robin_Watts> pdf_dump_obj and pdf_debug_obj should probably be merged. I forgot that I had committed one, so wrote another.12:15.35 
  <Robin_Watts> Why is journalling not always used? Well, journalling causes more memory use.12:16.15 
  <Robin_Watts> For stuff like pdfclean etc, it's completely pointless.12:16.34 
  <Robin_Watts> hence I felt it should be an option that we can turn on.12:16.47 
  <Robin_Watts> the page_next/page_prev stuff is a hangover from a previous version. I'll remove that. thanks.12:17.16 
  <Robin_Watts> The rest of your comments I'll need to think about 🙂12:18.43 
sebras ok. I'll come back in a bit to take a look at the journalling.12:25.02 
artifexirc-bot <Robin_Watts> There are certain types of save that break journalling, I think.13:25.26 
  <Robin_Watts> i.e. you can't undo across a save (in all circumstances, maybe not in any currently).13:25.55 
sebras ok. saving signed PDFs ought to be one per my investigation yesterday.13:25.56 
artifexirc-bot <Robin_Watts> Right.13:26.02 
  <Robin_Watts> So I'm thinking we need to disable journalling, do the save, then reinit it again.13:26.20 
sebras for me it makes sense that the undo history doesn't persist after saving, but I'm not sure how users would feel about that.13:26.37 
artifexirc-bot <Robin_Watts> which would address your issue, I think.13:26.44 
  <Robin_Watts> Have you been following the "background save" issue?13:27.23 
sebras yes, because if journalling isnot enabled pdf_begin_operation() will do an early return without error/warnings.13:27.48 
  background save issue? no.13:28.01 
artifexirc-bot <Robin_Watts> When MuPDF is running on a mobile platform, and it gets put into the background, it needs to do a 'save' of some kind, because it's possible that it might never get brought back into the foreground - it might get killed.13:28.33 
sebras right.13:28.50 
artifexirc-bot <Robin_Watts> Currently Paul and Fred are doing an incremental save to a temporary file, and when MuPDF restarts they reload it.13:28.55 
  <Robin_Watts> That has the effect of putting an extra incremental save section into each PDF each time that happens.13:29.23 
  <Robin_Watts> and with undo/redo we'd lose all that history.13:29.37 
  <Robin_Watts> So I have a vague plan (based off an idea inspired by paul) to add a new option to save, whereby we save out a version that has undo/redo data appended to it.13:30.13 
  <Robin_Watts> Such saves would need to be non-destructive in that they couldn't do renumbers etc.13:31.20 
  <Robin_Watts> I think that's all doable.13:31.32 
  <Robin_Watts> One case that is still unclear to me is the 'save for printing' case.13:31.48 
  <Robin_Watts> In order to print a file on Android/iOS, we have to save it as PDF and then send that off to the print server.13:32.08 
  <Robin_Watts> That needs to be a well formed PDF (i.e. we can't stick undo/redo stuff in there too).13:32.42 
  <Robin_Watts> and yet after the save, we can't afford to need to lose undo/redo etc, so it needs to be a "non destructive" save.13:33.19 
sebras you could format the undo/redo data as PDF comments using % and the PDF would still be well formed.13:34.15 
artifexirc-bot <Robin_Watts> While we have stuff open in memory, all the changes live in the "incremental" section (the last one in history in memory). When we do a save, we write that incremental section out.13:34.30 
  <Robin_Watts> I think we want an option that says "write this out, but then 'reopen' that section"13:34.48 
  <Robin_Watts> I think that should work, and the undo/redo history in memory should remain valid.13:35.19 
sebras for the backgrounding case, would it be possible to serialize our opened file to an Android Bundle some how? if that's the case there might not be a requirement on the format of the data (i.e. it doens't have to be a well formed PDF).13:35.23 
artifexirc-bot <Robin_Watts> For the backgrounding case, I was planning to copy the entire source PDF, and then follow it with the incremental data, and then follow it with the undo/redo data, followed by a "MUPDF-JOURNAL" tag or something.13:36.15 
sebras all of what you describe would conflict for PDFs with newly signed signatures..?13:36.31 
artifexirc-bot <Robin_Watts> Why?13:36.54 
sebras you just said we should disable journallying while saving..?13:37.16 
  and reinit after saving.13:37.20 
  wouldn't that effectively remove the undo history?13:37.33 
  and if this happens as part of printing a doc or backgrounding an app, then that is not desirable,right?13:38.10 
artifexirc-bot <Robin_Watts> For certain types of save, we throw the undo history away, yes (non-incremental saves at least).13:38.28 
  <Robin_Watts> For saves where we sign, then yes.13:39.01 
  <Robin_Watts> For other types (particularly for backgrounding saves), we don't.13:39.36 
sebras so when I open a document, I sign a signature and print it then after printing undo history is removed. is that ok?13:39.47 
  the reason the pdf objects are updated in memory as part of completing signatures is because I wanted the in-memory and on-disk objects to be identical.13:40.25 
artifexirc-bot <Robin_Watts> You open a document, and edit it. Journalling is on etc. When you sign it, you have to save it out. At that point undo history is lost.13:40.38 
sebras perhaps that is not necessary. but I haven't looked into it.13:41.27 
artifexirc-bot <Robin_Watts> I think that's fair enough (it'd be nice not to have to lose undo history on a sign, but I can't see how to do that at the moment).13:41.27 
  <Robin_Watts> We shouldn't lose history when we print though. That'd seem very odd to a user.13:41.50 
sebras well, we can remove updating the in-memory objects when completing signatures. that would help.13:42.07 
  the question is whether that is sane. I don't know.13:42.17 
  we do update the on-disk objects, perhaps that is enough, but it seems weird to me.13:42.38 
artifexirc-bot <Robin_Watts> If we do an "incremental save, and reopen the incremental section afterwards" version of 'save' then we can use that for printing without having to throw away undo/redo, I think.13:43.23 
sebras also, for the backgrounding have we considered using a Bundle? I'm not sure if there are size limitations for that. perhaps it is viable to stick the entire document in there. that way you hand over the handling of that background data to android.13:43.40 
artifexirc-bot <Robin_Watts> In general our undo/redo history says "starting from the saved file on disk, this is the list of changes".13:43.50 
  <Robin_Watts> sebras: That might solve for Android, but not for ios.13:44.14 
sebras does ios have something similar?13:44.25 
artifexirc-bot <Robin_Watts> And I'm not sure it neatly solves the problem.13:44.29 
sebras where would we save the temporary file (files if you have multiple documents open).13:45.02 
artifexirc-bot <Robin_Watts> All this is stuff I will ponder on (and forget, no doubt) over the holiday.13:45.07 
  <Robin_Watts> sebras: Fred/Paul already have such a location.13:45.17 
  <Robin_Watts> That's an app level decision, not a mupdf library level decision, fortunately.13:45.39 
artifexirc-bot <Robin_Watts> is called for lunch...13:45.46 
  <Robin_Watts> There are updated versions of the commits to address most of your concerns online now.13:46.33 
sebras ok, thanks.13:46.46 
  I'll continue reviewing the first iteration of the journalling stuff though.13:47.03 
artifexirc-bot <Robin_Watts> Sure.13:47.08 
sebras I've already started here. :)13:47.15 
artifexirc-bot <Robin_Watts> The simplification to the ap stuff looks to have caused problems. I'll look into why after lunch.13:47.31 
sebras current->next->prev->current->prev->next.......13:47.37 
  in prepare_object_for_alteration() won't if (doc && doc->journal && doc->journal->nesting == 0) cause coverity to complain that doc might be NULL but we dereference doc a few lines above?13:50.29 
artifexirc-bot <Robin_Watts> sebras: Yes, I couldn't see a nice formulation for the linked list stuff this time 😦14:54.30 
  <Robin_Watts> and I agree about the doc thing. Will fix.14:54.43 
sebras in add_fragment() after it has discard the entry list in entry->next, isn't there a risk that doc->journal->tail is stale?15:16.49 
artifexirc-bot <Robin_Watts> yes.15:18.02 
sebras in add_fragment() in fz_catch() you drop copy_stream, but in the calling function prepare_object_for_alteration() you also drop copy_stream. in the catch clause. would it make sense to assume that add_fragment() takes ownership of copy and copy_stream and thus require add_fragment() itself to either store the references in the added fragment or drop them, that way resources handling might be clearer.15:19.36 
  add_fragment_drop() perhaps?15:19.42 
artifexirc-bot <Robin_Watts> The issue is that I might throw before I get to the add_fragment call, and I still need to drop them in the caller.15:21.40 
  <Robin_Watts> Better if add_fragment never drops, I think.15:22.33 
sebras ok, then it needs to keep the object/stream instead.15:22.47 
artifexirc-bot <Robin_Watts> No. It can safely take it, because it's the last thing called.15:23.10 
  <Robin_Watts> If it throws it hasn't kept references. If it doesn't it has.15:23.33 
sebras oh! it is the orig object that is dropped.15:23.40 
  in always.15:25.48 
artifexirc-bot <Robin_Watts> Yes.15:26.52 
  <Robin_Watts> I have a fixed version here.15:26.58 
sebras in pdf_begin_operation() can't we do entry->title = fz_strdup(operation_) at the end and thus not having to care about the fz_free() in fz_catch() or the operation variable at all?15:31.05 
artifexirc-bot <Robin_Watts> If the title strdup fails, then we'd need to unlink the new block before we threw, right?15:32.10 
  <Robin_Watts> That seems much more work that freeing operation.15:32.22 
  <Robin_Watts> That seems much more work than freeing operation.15:32.33 
sebras in the same funciton, in the comment at the top of fz_try(), maybe mention that the journal entries are discarded in add_fragment() (i.e. not in _this_ function)_15:32.42 
  ah, yes. you're right.15:33.05 
  the reason for wanting that pointer in the comment is that you talk about something that happens elsewhere, so let's be explicit about where.15:33.46 
artifexirc-bot <Robin_Watts> right. will fix that in a mo.15:34.15 
sebras @Robin_Watts is it correct to, upon exception, decrement nesting both in fz_catch() of pdf_begin_operation() and in pdf_end_operation() that is called in fz_always() by the caller? don't we decrement test twice in that case?15:38.51 
artifexirc-bot <Robin_Watts> If pdf_begin_operation throws, we shouldn't call pdf_end_operation, cos the operation never began.15:41.01 
  <Robin_Watts> Where are you seeing that?15:41.05 
sebras e.g. in pdf_set_text_field_value()..?15:41.39 
  and toggle_check_box().15:42.00 
  and pdf_sign_signature().15:42.18 
  most occurrences of pdf_begin_operation() is outside fz_try, and then pdf_end_operation() wouldn't be called.15:42.40 
  that makes sense to me.15:42.46 
artifexirc-bot <Robin_Watts> I'm confused.15:43.10 
  <Robin_Watts> pdf_sign_signature is bad, the others look OK to me.15:44.19 
  <Robin_Watts> Wait...15:46.36 
  <Robin_Watts> go back to add_fragment15:46.43 
sebras in toggle_check_box() we call pdf_beign_operation() inside fz_try(), that means that pdf_begin_operaiton will increment nesting, right? if then e.g. fz_malloc_struct(pdf_journal_entry) fails pdf_begin_operation() will in fz_catch() decrement nesting, then we return back to toggle_check_box() and end up in fz_always() that calls pdf_end_operation() which, again will decrement nesting.15:46.43 
  isn't that a problem?15:47.01 
artifexirc-bot <Robin_Watts> Not in what I have here.15:47.10 
  <Robin_Watts> How old is the version you are looking at?15:47.38 
sebras since I started reviewing it.15:47.52 
artifexirc-bot <Robin_Watts> What's the date on the commit?15:48.16 
sebras AuthorDate: Fri Nov 27 13:11:10 2020 +000015:48.57 
  CommitDate: Mon Dec 14 12:49:17 2020 +000015:49.07 
artifexirc-bot <Robin_Watts> In the version I have here, the begin is outside the fz_try() and I haven't changed that for a while...15:49.20 
sebras ok, I'll fetch and update.15:49.38 
artifexirc-bot <Robin_Watts> discard_journal_entries does not affect entry->tail.15:50.00 
sebras no, I mean doc->journal->tail15:50.22 
artifexirc-bot <Robin_Watts> Right.15:50.37 
  <Robin_Watts> Ok, I've just pushed a version with fixes in.15:51.52 
  <Robin_Watts> If you're going to restart, you probably want to restart from this.15:52.03 
  <Robin_Watts> And my simplified AP stuff just passed. Phew.15:52.46 
  <Robin_Watts> Oh, we're mising the meeting.15:53.27 
sebras but even in the latest 5a5f9be080bd0b087483fb42faf2036c8a93e473 there is a hunk where you change toggle_check_box() and adds a pdf_begin_operation() inside a fz_try while fz_always calls pdf_end_operation?15:53.48 
  meeting?15:53.53 
artifexirc-bot <Robin_Watts> Tuesday meeting.16:04.21 
  <Robin_Watts> started 34 minutes ago in #artifex_meeting16:04.33 
sebras yeah, I'm there, now.16:18.52 
  @Robin_Watts so I still see issues with toggle_check_box().16:37.48 
artifexirc-bot <Robin_Watts> OK. Looking...16:37.55 
  <Robin_Watts> What do you see that's wrong?16:38.39 
sebras fz_try() { pdf_begin_operation(); .... } fz_always() { pdf_end_operation() }16:39.18 
artifexirc-bot <Robin_Watts> ok, I don't see that. I wonder if I've modified it in a later commit.16:39.38 
sebras I'm on 0f00190fc16:39.50 
artifexirc-bot <Robin_Watts> yes, I have modified it later... I wonder if that's a case of me having squashed a fix back incorrectly...16:40.47 
sebras aha! no wonder we're not in agreement. I'm not reviewing the latest code, just that commit. :)16:41.17 
artifexirc-bot <Robin_Watts> I'll try and fix that.16:41.47 
sebras it might useful to consistently call pdf_begin*_operation() outside of fz_try(), e.g. even in pdf_update_page() even if implicit operations cannot cause that same type of issue.16:43.35 
  @Robin_Watts you've modified pdf_set_text_field_value() in a later commit too (same as toggle_check_box())16:46.25 
artifexirc-bot <Robin_Watts> Yeah, I'm pulling various things back.16:46.49 
sebras I took me sometime to realize that the normal exit from pdf_end_operation() is after the entry == NULL || entry->head != NULL check, and that everything below is for when nothing really changed since pdf_begin_operation()16:53.23 
artifexirc-bot <Robin_Watts> Yeah. If memory serves, there is a comment there.16:53.54 
  <Robin_Watts> OK, I think I've pulled stuff back properly.17:09.13 
  <Robin_Watts> and I think the latest version (that I just pushed) is consistent with calling it outside try/catch.17:13.19 
  <Robin_Watts> There is no comment. I will add one.17:13.53 
  <Robin_Watts> Added.17:16.51 
sebras fetches17:34.54 
artifexirc-bot <Robin_Watts> Is there a prefix of commits that can be pushed?17:35.32 
sebras yes17:36.40 
  LGTM MSVC Extract: Add join.c to project.17:36.42 
  LGTM Add Memento_blockInfo(void *addr).17:36.48 
  "Bug 703266: Fix component decode of function based type 5 shadings." is new, I haven't review that one I think?17:37.04 
artifexirc-bot <Robin_Watts> That is new, yes.17:37.23 
  <Robin_Watts> I can rebase that after stuff you've reviewed if needed.17:37.37 
  <Robin_Watts> It gives zero diffs in the cluster and fixes bug 70326617:38.00 
sebras I think that might be smart. because there are later commits that you've touched up that I'm probably ok with.17:38.06 
artifexirc-bot <Robin_Watts> sure. You tell me which ones you're happy with and I'll juggle.17:38.45 
sebras LGTM Add fz_malloc_struct_array function.17:39.43 
  LGTM Remove pdf_annot->ap.17:41.34 
  I will probably need ator to understand has_new_ap och needs_new_ap. we can sort that out in january (I've put a reminder in my calendar)17:42.01 
  LGTM Renumber page and annotation objects as part of saving.17:44.37 
artifexirc-bot <Robin_Watts> Urm, has the cluster not run any mupdf git jobs since Nov 18th?17:49.23 
sebras @Robin_Watts the fz_stdods() fails to build for me.17:50.05 
  and sorry to be brining this up. shoudl this debug message handling be in something like the fz_warn_context? I'm not sure if that is better or worse. I just remember, and I ought to have thought about it earlier. :-/17:51.27 
artifexirc-bot <Robin_Watts> This is the perfect time to talk about it.17:51.48 
  <Robin_Watts> The warn context just has stuff for warns.17:52.03 
  <Robin_Watts> This is independent of the exception stuff.17:52.14 
sebras well fz_warn_context is also independnet of fz_error_context. i.e. fz_error_context has a stack, while fz_warn_context does not.17:55.33 
  I'm not sure whether a printf-alike callback is any better than a fz_output, but it might be worthwhile considering at least.18:01.09 
artifexirc-bot <Robin_Watts> It would be nicer if the warn stuff went to the stddbg.18:01.46 
sebras is it the case that the differen between warnings and your debug messages is just "warning:" at the beginning of the line?18:03.56 
  maybe it should be fz_message_context and a debug level (WARNING, INFO, DEBUG, etc...)?18:04.29 
  I'm not sure what is the best approach I just see similarities.18:04.39 
artifexirc-bot <Robin_Watts> The main use for my stuff is that when we enable debugging (or call debugging printfs from the debugger) I want to be able to see the damn things.18:07.08 
  <Robin_Watts> Sending stuff to stderr is no use for windows graphical mode apps.18:07.18 
sebras nope, I get that. and on linux we only have stderr as an option.18:07.58 
artifexirc-bot <Robin_Watts> And apps on stuff like android or ios could invent their own logging things.18:08.41 
sebras but then again that's why we have fz_warn_context with a callback so that apps can choose to accumulate messages and display theym in a useful way (stderr/dialog box/log file/whatever)18:08.43 
  same with fz_error_context, but that also involves the exception stack of course.18:09.06 
  I'm specifically thinking about the fz_{error,warn}_context->print() callbacks18:09.34 
artifexirc-bot <Robin_Watts> Well, the printing doesn't...18:09.35 
  <Robin_Watts> Right.18:09.41 
  <Robin_Watts> rather than having a print callback for each one, we could use a generic 'print' callback for warn/error/stddbg.18:10.24 
  <Robin_Watts> and I could implement a callback to go to OutputDebugString.18:10.53 
sebras in that case we need a level argument so the app can choose what messages are important.18:11.04 
  becuase they might care about errors but not warnins?18:11.32 
artifexirc-bot <Robin_Watts> BUT... that would still leave me unable to call standard utility functions (like fz_print_obj) with that.18:11.32 
  <Robin_Watts> Yeah. I'm saying I don't think I care about the 3 things all being unified.18:11.53 
sebras oh? I didn't pick up on that.18:12.16 
artifexirc-bot <Robin_Watts> But I'm happy to leave this as one on the "It looks technically fine, but let's talk to tor about what he things" stack.18:12.31 
  <Robin_Watts> But I'm happy to leave this as one on the "It looks technically fine, but let's talk to tor about what he thinks" stack.18:12.36 
sebras if we do then "Add journalling API to MuPDF." either needs to wait or keep the debugging waiting.18:13.33 
  I need to review the pdf_undo()/pdf_redo() functions too before I LGTM that one though.18:14.25 
artifexirc-bot <Robin_Watts> I was thinking that all the big ones would need to wait, but it'd be good to get them to the stage where we are both happy.18:16.05 
  <Robin_Watts> Balls. I think the mupdf one was broken 😦18:16.22 
  <Robin_Watts> Balls. I think the memento one was broken 😦18:16.30 
  <Robin_Watts> There will now be some more swearing at git and the cluster.18:16.45 
sebras yeah, and some swig stuff for cgdae.18:17.31 
artifexirc-bot <Robin_Watts> I've dropped cgdae an email. I'm sure it'll be a simple fix (or he may already have fixed it and it might be in the queue)18:39.27 
 <<<Back 1 day (to 2020/12/14)Forward 1 day (to 2020/12/16)>>> 
ghostscript.com #ghostscript
Search: