| | <<<Back 1 day (to 2020/12/10) | Fwd 1 day (to 2020/12/12)>>> | 20201211 |
| artifexirc-bot | <Robin_Watts> @sebras If you are looking at my commits, ignore the last 2 for now. | 15:59.20 |
| | <Robin_Watts> I've spotted a problem which means I need to rework the last one. | 15:59.33 |
| | <Robin_Watts> I've spotted a problem which means I need to rework the last but one. | 15:59.43 |
| sebras | @Robin_Watts Re: "MSVC Extract: Add join.c to project." | 16:10.22 |
| | shouldn't the Filter be "Source Files\extract"? does it matter? | 16:10.36 |
| artifexirc-bot | <Robin_Watts> sebras That doesn't matter, AIUI. As long as we are consistent with the other files. | 16:11.06 |
| sebras | @Robin_Watts the other's use Source Files\extract... | 16:11.28 |
| artifexirc-bot | <Robin_Watts> I'll double check that in a mo, thanks. | 16:11.40 |
| sebras | ok. /me is continuing. | 16:11.51 |
| artifexirc-bot | <Robin_Watts> You are right. | 16:11.57 |
| | <Robin_Watts> Will fix. | 16:12.00 |
| sebras | @Robin_Watts Re: "Add Memento_blockInfo(void *addr)." put showBlockInfo() inside ifdef MEMENTO_DETAILS, or else showBlockInfo() is defined when MEMENTO_DETAILS is not set, and it will try to call the undeclared showInfo(). | 16:15.23 |
| artifexirc-bot | <Robin_Watts> Right, will fix. ta. | 16:15.41 |
| sebras | maybe MEMENTO_DETAILS is never unset? I don't know. :) | 16:16.14 |
| artifexirc-bot | <Robin_Watts> It's never unset unless someone deliberately unsets it 🙂 | 16:16.39 |
| sebras | should the comparison MEMBLK_POSTPTR(b) < arg be <= ? | 16:17.19 |
| artifexirc-bot | <Robin_Watts> We're checking for arg not being inside the block. | 16:17.51 |
| | <Robin_Watts> so it's right as is, but arguably we could want to spot pointers that point to the end of the block. | 16:18.16 |
| sebras | agreed, I'm not sure where POSTPTR points though. is that at the last byte, or at the next byte? | 16:18.27 |
| artifexirc-bot | <Robin_Watts> It's the first byte of the postblock. | 16:19.44 |
| | <Robin_Watts> so it's not accessible to the user of memento. | 16:20.00 |
| | <Robin_Watts> (Memento allocates larger than required, and has a pre block, including guard words, then the actual user accessible block, then a post block with some more guard words) | 16:22.06 |
| | <Robin_Watts> POSTPTR points to the start of the guard words in the post block. | 16:22.19 |
| | <Robin_Watts> Updated those 2 commits. | 16:22.28 |
| sebras | right, then I think <= might be better. | 16:22.40 |
| | otherwise the arg might point to the first byte of the postblock and we'll call showBlockInfo(), and I think that's not intentional. | 16:23.42 |
| artifexirc-bot | <Robin_Watts> OK. | 16:24.12 |
| sebras | if I've misunderstood the situation I need more explanation. :) | 16:24.30 |
| artifexirc-bot | <Robin_Watts> No, I think you've grasped it. | 16:24.56 |
| | <Robin_Watts> updated and pushed. | 16:25.50 |
| sebras | @Robin_Watts Re: "Add fz_malloc_struct_array function." I don't oppose fz_malloc_struct_array() necessarily, but you never use it in any commit on robin/master, so why do we need it? | 16:25.54 |
| artifexirc-bot | <Robin_Watts> I needed it briefly. | 16:26.06 |
| | <Robin_Watts> but I think it's something that'd be good to have. I bet there are cases where we could have done with it. | 16:26.26 |
| | <Robin_Watts> Possibly I should look for such cases as part of that commit. | 16:26.37 |
| sebras | or in a separate succeeding commit. | 16:27.27 |
| | @Robin_Watts in "Remove pdf_annot->ap." there is commented out code in pdf_update_appearance(). | 16:28.02 |
| artifexirc-bot | <Robin_Watts> oops. will fix. | 16:28.20 |
| | <Robin_Watts> I'm doing well with these commits so far 🙂 | 16:28.30 |
| sebras | no worries. :) | 16:28.58 |
| artifexirc-bot | <Robin_Watts> For some reason, everything you say is appearing on discord twice... | 16:29.22 |
| sebras | I blame artifexirc-bot. | 16:32.06 |
| artifexirc-bot | <Robin_Watts> That's better. | 16:33.11 |
| | <Robin_Watts> I restarted matterbot on casper. No idea why it was duplicating stuff. | 16:33.33 |
| | <Robin_Watts> of course, I've not heard anything since. @sebras Are you still seeing this? | 17:08.42 |
| sebras | @Robin_Watts I am looking at the next annot patch. | 17:15.22 |
| | has_new_ap confuses me. | 17:15.25 |
| artifexirc-bot | <Robin_Watts> Confuses the hell out of me too. | 17:15.34 |
| sebras | isn't has_new_ap supposed to be set in update_checkbox_selector()? | 17:15.43 |
| | we're changing state after all. | 17:15.52 |
| | or is needs_new_ap supposed to be set? | 17:16.14 |
| artifexirc-bot | <Robin_Watts> update_checkbox_selector is only ever called from itself, or from update_field_value. | 17:16.54 |
| sebras | to me it seems pdf_set_field_value() often ought to set has_new_ap? | 17:17.00 |
| artifexirc-bot | <Robin_Watts> update_field_value calls pdf_field_mark_dirty | 17:17.02 |
| | <Robin_Watts> That calls pdf_dirty_obj, and that sets needs_new_ap | 17:17.27 |
| sebras | how does has_new_ap differ from being marked dirty? | 17:17.34 |
| artifexirc-bot | <Robin_Watts> That calls pdf_dirty_obj, and that sets the object to be dirty. | 17:17.52 |
| | <Robin_Watts> I think the dirty bit really needs a better name, cos it's unclear to me. | 17:18.16 |
| sebras | oh needs_new_ap is due to JS events! | 17:18.40 |
| artifexirc-bot | <Robin_Watts> pdf_update_appearance does: if (is dirty) needs_new_ap = 1; | 17:18.55 |
| sebras | and has_new_ap is the consequence of pdf_update_appearance() being called. | 17:19.00 |
| | I still wonder what is the conceptual difference between pdf_toggle_widget() and pdf_set_field_value() | 17:20.09 |
| | the former set s has_new_ap | 17:20.20 |
| | but pdf_set_field_value() does not, instead it marks the field as dirty. | 17:20.45 |
| | wouldn't both toggle the state and change the appearance stream? | 17:21.14 |
| artifexirc-bot | <Robin_Watts> Have I changed any of that? | 17:22.49 |
| | <Robin_Watts> gitk | 17:25.30 |
| | <Robin_Watts> It's possible that the addition to pdf_toggle_widget is wrong. I'll check that. | 17:26.46 |
| sebras | not necessarily you, but I don't fully understand has_new_ap and needs_new_ap and the dirty flag usage, so since you're changing parts of that I need to understand what is actually happening. | 17:29.15 |
| artifexirc-bot | <Robin_Watts> I'm not really changing that. | 17:30.41 |
| | <Robin_Watts> All I'm changing is that rather than keeping annot->ap, I dict_get(annot->obj, PDF_NAME(AP)) each time. | 17:31.13 |
| | <Robin_Watts> (That's simplifying it, but that's the intent) | 17:31.27 |
| sebras | pdf_update_appeance() used to set has_new_ap = 1 whenever annot->ap and ap_n difference, I'm not convinced it will do the same now. but maybe that is intentional? | 17:31.47 |
| artifexirc-bot | <Robin_Watts> Ah, yes, I remember now. | 17:32.12 |
| sebras | yes, I know about pdf_annot_ap(), those changes all seem reasonable, but there are related changes too. :) | 17:32.17 |
| artifexirc-bot | <Robin_Watts> We used to compare annot->ap == ap_n to spot that the appearance had changed. | 17:32.38 |
| | <Robin_Watts> We can't do that because annot->ap ain't held anywhere. | 17:32.55 |
| | <Robin_Watts> I believe the only case that can happen in is for buttons. | 17:33.11 |
| | <Robin_Watts> which is why I now set has_new_ap when the widget is toggled. | 17:33.37 |
| sebras | so has_new_ap indicates that AP/N has been updated? | 17:34.43 |
| | that seems to match pdf_update_signature_appearance(). | 17:35.00 |
| artifexirc-bot | <Robin_Watts> I believe so, but that is based upon my own dodgy understanding. | 17:38.48 |
| sebras | hm. why doesn't toggle_check_box() set /AS? | 17:38.56 |
| | if it doens't wouldn't we detect it incorrectly now? since we rely on /AS in pdf_annot_ap()? | 17:39.36 |
| artifexirc-bot | <Robin_Watts> toggling check boxes changes between one of a set of pregenerated appearance streams. | 17:40.11 |
| | <Robin_Watts> i.e. we swap N from /Off to /Mr or something | 17:40.29 |
| sebras | right. | 17:40.35 |
| | but toggle_check_box() only updates the toggled value in /V it seems? | 17:41.12 |
| artifexirc-bot | <Robin_Watts> Ah, because we set has_new_ap, the pdf_update_appearance sets AS ? | 17:41.42 |
| | <Robin_Watts> (The short answer is I don't know. I was trying to do the minimal changes to remove ->ap, and that didn't involve me having to change any of that) | 17:43.26 |
| sebras | except in pdf_toggle_widget/). | 17:45.19 |
| | and pdf_update_appearance(). | 17:45.35 |
| | so most of the commit looks fine to me, but those two parts I don't yet understand the consequences of. | 17:45.52 |
| | that being said, the changes might be fine! | 17:46.20 |
| artifexirc-bot | <Robin_Watts> sebras: In pdf_update_appearance, the old code said: "if annot->ap ain't what I just read from the document, update ap to be what I just read, and set annot->has_new_ap" | 17:48.51 |
| | <Robin_Watts> hence cos I removed ->ap, I removed all that and just set annot->has_new_ap directly. | 17:49.07 |
| | <Robin_Watts> Nowhere have I changed anything to do with how AS is set. | 17:49.18 |
| | <Robin_Watts> @sebras If we get to the point where stuff seems plausible to both you and I, but we want to @ator to look when he returns, that's fine. | 17:52.26 |
| | <Robin_Watts> That's still progress, as far as I am concerned, cos your reviews catch useful stuff! | 17:52.47 |
| sebras | my question about AS is due to me trying to understand how this is supposed to work. it is unrelated to your commit. :) | 17:53.55 |
| | when you say you set has_new_ap directly, one example of this is then in pdf_toggle_widget(), right? | 17:54.33 |
| | because your commit doesn't add any assignment of has_new_ap in pdf_update_appearance(). | 17:54.59 |
| artifexirc-bot | <Robin_Watts> That's *the* example yes. | 17:55.01 |
| | <Robin_Watts> I trade an assignment in pdf_update_appearance for one in pdf_toggle_widget. | 17:55.21 |
| sebras | so has_new_ap is not == 1 earlier. | 17:55.57 |
| | ok. | 17:56.01 |
| | @Robin_Watts I probalby need some time to look at this. | 17:57.29 |
| artifexirc-bot | <Robin_Watts> Sure. Much appreciated. | 17:57.52 |
| | <Robin_Watts> I've updated the commits with fixes for all your comments so far I think. | 17:58.10 |
| | <Robin_Watts> And broadly stuff seems to be working. | 17:58.17 |
| | <Robin_Watts> I have a few diffs in cluster tests that I am looking into. | 17:58.29 |
| | <<<Back 1 day (to 2020/12/10) | Forward 1 day (to 2020/12/12)>>> | |
Log of #mupdf at irc.freenode.net.