| | <<<Back 1 day (to 2018/08/09) | 20180810 |
| sebras | tor8 (for the logs): oh, I didn't remember that command-line variables overrides variables set with := | 05:35.38 |
| | tor8 (also for the logs): += SANITIZE_XFLAGS is not enough, because -fsanitize=memory and -fsanitize=address etc cannot be combined, it is one or the other. with some more elaborate ifdeffing with SANITIZE_XFLAGS to let it entirely replace SANITIZE_FLAGS that might work. | 05:36.49 |
| | tor8: so I read that I need to use clang to link, hence I set LD=clang at the command line, but clang doesn't support -b binary used in OBJCOPY_CMD. when retesting without setting LD=clang now it worked, at first I was puzzled then I found out that we use $(CC) for linking, so I actually do not need to set LD=clang, ok. | 05:48.26 |
| | that means that make CC=clang CXX=clang SANITIZE_FLAGS=-fsanitize=memory build=sanitize ought to work fine, great. :) | 05:51.10 |
| paulgardiner | tor8: please can you ping me when "Don't trigger the keystroke event when clearing a form field" and "Respect NoRotate flag for icon-based annotations" hit the main repo. | 09:56.02 |
| | Also Fred suggested a change to util.js that we should take on. Not sure what happened with that. | 09:56.28 |
| tor8 | paulgardiner: I was on vacation :) | 10:02.23 |
| | paulgardiner: what was fred's suggestion? | 10:05.31 |
| paulgardiner | Checking that all of "val" is used up at the end of AFSpecial_KeystrokeEx, otherwise we silently truncate text that fails because of extra chars tagged on the end | 10:07.32 |
| | Test that would fail the match, I should say | 10:08.25 |
| | Looking at the other similar functions, I think Fred's version would be correct | 10:09.06 |
| tor8 | paulgardiner: which do you think is best, checking the string length in pdf_text_widget_set_text before calling run_keystroke, or doing the test for clearing a field inside run_keystroke itself? | 10:12.01 |
| paulgardiner | tor8: I struggle to have a preference. Probably I prefer yours a little. | 10:13.01 |
| | I certainly prefer *str == 0 over strlen(str) == 0 :-) | 10:13.34 |
| tor8 | paulgardiner: okay. I'll push my variant then. just clustering now. | 10:18.55 |
| paulgardiner | Great ta | 10:19.04 |
| | Not that there was any hurry for these changes - just wanted to be sure I didn't overlook when they hit the main repo | 10:19.51 |
| tor8 | they were reviewed yesterday, so I should get around to pushing them before the weekend after which I'll forget everything again :) | 10:20.42 |
| paulgardiner | :-) | 10:24.43 |
| tor8 | paulgardiner: oh rubbish... something in those commits broke the placement of stamp annotations | 10:35.41 |
| paulgardiner | Bah! That's a pain. | 10:36.24 |
| tor8 | at least I know what to do the rest of the day then :) | 10:36.50 |
| paulgardiner | tor8: how do we pick up update for one page caused by changes to another: e.g., I have a form where filling in a field on Page 1 provides the value for similar fields on all pages. | 12:26.12 |
| | s/update/the updates/ | 12:29.25 |
| tor8 | paulgardiner: I would expect that to work already, the obj->dirty flag is set when the javascript pokes a new field value | 12:35.48 |
| | and that should be picked up in pdf_update_annot | 12:35.58 |
| paulgardiner | Yeah, I'm currently calling pdf_update_page only for the page with the explicitly changed field on, so hopefully doing so for all displayed pages should fix the problem I'm seeing. | 12:38.30 |
| tor8 | yeah. calling pdf_update_page when nothing has changed should be a very quick operation | 12:39.23 |
| paulgardiner | I asked mainly because I thought I remembered there being a problem if when working on a single page, you used two fz_page objects one for making the change and the other for picking up the update. I can't see why that would be problematic if the multiple page case works. | 12:39.39 |
| sebras | tor8: you change at the top of tor/master LGTM | 12:40.05 |
| tor8 | yeah, two fz_page's for the same page would be problematic | 12:40.05 |
| paulgardiner | Yeah, why is that? | 12:40.17 |
| tor8 | because each page has its own set of loaded annotations and cached state... | 12:40.31 |
| | but that *could* be fixed by making all pdf_page objects singletons -- always return the same pdf_page object for each fz_load_page | 12:41.03 |
| | but then we'll need to keep a list of currently open pages in the document to look for if you have already opened it | 12:42.06 |
| sebras | tor8: but that would mean that a change to one is neccessarily a change to them all. do we know that this os always desired? | 12:44.08 |
| tor8 | both pdf_page's would represent the same page, I can't imagine any situation where you'd want to load the same page but treat them as separate things | 12:45.19 |
| | the stuff in the pdf_page is only a way to access the underlying pdf_obj structures using a convenient C interface, with some caching for performance | 12:45.46 |
| | and it's the caching that causes problems if you load the same page twice and poke at the pdf_page cached structures in one and expect them to show up in the other. | 12:46.16 |
| | either we don't do that, or we have to go to pains to ensure that a stupid client that calls fz_load_page twice without knowing it's already got the page open elsewhere gets the same pdf_obj struct both times | 12:47.01 |
| | IMO it's easier and generally more sound architecture to structure a client so it doesn't open a page twice, than futz around making workarounds for "bad" behaviour in the library | 12:47.48 |
| paulgardiner | In muso there a few cases where I have multiple fz_pages for a single page, but I could possibly avoid it. One reason is because of multiple views on the same page, another is loading page for search independently of what's visible. | 12:47.50 |
| tor8 | someone will need to keep track of what's open and closed, either we move that magic into the fz_document interface or the client needs to do it | 12:48.26 |
| | say, what should the library do if you decide to delete a page while it's open, or insert a page in the middle, etc | 12:50.03 |
| | we'd need to take care of all these corner cases if we were to add some memoization thing for loading pages | 12:50.38 |
| | or have a linear search through all open pages, or have a big fat array of fz_page pointers in the document, wasting memory | 12:51.12 |
| | paulgardiner: why would multiple views on a page require multiple fz_load_pages? | 12:51.59 |
| | paulgardiner: I assume the easiest solution would be for either muso or the fz_document to have an array of fz_page objects that are loaded | 12:52.37 |
| | but then we need to be able to free it if the last instance is dropped | 12:53.07 |
| | so maybe a linked list in the fz_document is preferable | 12:53.18 |
| paulgardiner | tor8: the use with multiple views is simply that I haven't run into sufficient problems with it to address it. :-) | 13:03.00 |
| tor8 | paulgardiner: I'm looking into a simple singleton-ifying list in fz_document | 13:03.19 |
| paulgardiner | I could quite easily create an fz_page cache | 13:03.30 |
| | tor8: that may well be worth doing, just because it might simplify things for all clients. | 13:04.09 |
| | ... well not for the ones that have taken the trouble to avoid the problem already. | 13:04.32 |
| | Hang on. Wouldn't that require reference counted fz_pages? | 13:05.13 |
| | ... or are you thinking keep them until the doc is closed? | 13:05.40 |
| tor8 | fz_pages are already reference counted. | 13:05.51 |
| paulgardiner | Oh okay | 13:06.17 |
| | Come to think of it, there are still difficulties in clients even with the singletons. The update for a page have to be sent to all views on that page | 13:07.32 |
| | ... but then again having different fz_pages for a single page doesn't help with that, at least how thing are now. | 13:11.08 |
| tor8 | no, I think multiplexing multiple views onto one page will need some more fancy propagation | 13:12.44 |
| paulgardiner | I'm now thinking to ensure updates work correctly, I will need to do the same structuring that I'd use to ensure only one fz_page per page, so the singleton thing may not help. | 13:12.51 |
| tor8 | likely a wrapper object in muso that has a list of 'currently viewing this page' views | 13:13.06 |
| paulgardiner | Yeah | 13:13.16 |
| tor8 | paulgardiner: right. if you need it, I've put a WIP commit adding a singleton tracker on tor/master | 13:13.59 |
| paulgardiner | Okay ta | 13:14.27 |
| tor8 | completely untested you understand :) | 13:15.42 |
| paulgardiner | I think likely I wont use it. Apologies for provoking you to do it. I think it's time I made sure this all worked properly in muso. I think I can make it so that the obj-C level page objects can have multiple instances per page, and each will handle update correctly, but in the implementation of those ensure that a singe fz_page per page is used. That way I don't need to change anything... | 13:21.57 |
| | ...outside the low level obj-C MuPDFDKLib | 13:21.58 |
| tor8 | paulgardiner: not a problem. I might even decide to keep the patch anyway, since it certainly shouldn't do much harm and it's a small simple patch. | 13:42.58 |
| paulgardiner | Yeah, I can't see how it can other than help | 13:43.26 |
| tor8 | only issue is what happens when we start inserting and deleting and moving pages around, if we add such APIs | 13:43.45 |
| | which is why it's still a WIP | 13:44.21 |
| paulgardiner | Oh, turns out I already distribute the updates correctly. I've forgotten how this all works. | 13:51.37 |
| sebras | tor8: have you looked at fred/master? it looked reasonable to me. | 14:22.26 |
| tor8 | sebras: already pushed in commit 199668e93 | 14:37.33 |
| fredross-perry | sebras, tor8 - while you're at it, can you take a look at fred/forms2? It's the stuff I've done to get form editing working in SO. | 14:38.11 |
| tor8 | fredross-perry: I think all changes except the "WIP working on form filling" are already on master | 14:38.44 |
| | fredross-perry: if you want I can take a look at that commit too (but it'll have to wait till next week) | 14:39.06 |
| fredross-perry | ok, thanks. I don't mean to push it just yet, 'cause I know you' (or sebras) will have a bunch of comments. | 14:39.57 |
| sebras | tor8: oh, I wasn't aware that you pushed it sorry. :) | 15:46.50 |
| | Forward 1 day (to 2018/08/11)>>> | |
Log of #mupdf at irc.freenode.net.