| | <<<Back 1 day (to 2018/06/28) | 20180629 |
| paulgardiner | tor8: I think I now see why we get different behaviour in muso and mupdf-gl. In mupdf-gl pdf_update_page is run between pointer down and pointer up, so you detect the changes between the checkbox's down and up state on both occasions. In muso, pdf_update_page is called only after both the down and the up events are sent, so it needs to detect the checkbox going from off state to on state. I... | 09:20.50 |
| | ...haven't yet worked out why that is failing. | 09:20.51 |
| tor8 | paulgardiner: huh, that's a weird one. | 09:22.27 |
| | I've figured out the red background (use the /MK/BG property, not the /C) | 09:22.50 |
| paulgardiner | Ah right. That sounds familiar | 09:23.06 |
| | Presumably we are update the as field when a checkbox is clicked and not just changing the appearance stream | 09:24.44 |
| | the "AS" field, I mean | 09:25.07 |
| tor8 | oh, right. I have a hunch. | 09:25.36 |
| | on the 'down' you update the state to on/off on the clicked checkbox, and the appearance shown is /D for clicked | 09:26.06 |
| | on the 'up' you update the state on the other radioboxes in the group | 09:26.35 |
| | and the appearance shown is /N for the clicked one | 09:26.46 |
| | no, nevermind my hunch. | 09:27.42 |
| paulgardiner | Yesterday, as well as having missing changes I had many extra ones. That was just because I hadn't taken on all the commits from your branch. So at least I have just a problem of missing ones today | 09:30.33 |
| tor8 | paulgardiner: yeah, I fixed a couple of false positives. | 09:30.54 |
| | paulgardiner: try top of tor/master | 09:32.10 |
| | there's one new commit there | 09:32.16 |
| | it still doesn't explain why mupdf-gl would work while yours doesn't :( | 09:32.57 |
| paulgardiner | I think it does. Look at line 1283 of pdf-appearance.c | 09:36.49 |
| tor8 | I've logged where it detects the changes, and it is indeed in the pdf_update_state 'hotspot' detection | 09:37.00 |
| | why the hotspot would affect the non-clicked button mystifies me though | 09:37.38 |
| paulgardiner | In muso, I'm never calling pdf_update_page and hence pdf_update_annot with the pointer down. | 09:37.39 |
| | It's the clicked button that is failing for me. | 09:38.18 |
| tor8 | paulgardiner: oh! | 09:38.41 |
| | are all buttons failing for you? | 09:38.48 |
| | I thought it was just that after checking Single, it would not uncheck when you click Married later. | 09:39.05 |
| paulgardiner | Yes. Sorry, obviously didn't make that clear. | 09:39.06 |
| | Well not quite: for radio buttons, I get update for the one going unchecked. | 09:39.30 |
| | I'm just about to try out your new commit, but I'm wondering how it relates to line 1292 of pdf-appearance.c | 09:40.38 |
| tor8 | paulgardiner: weird. if I uncomment the hotspot handling everything breaks here too. | 09:41.02 |
| paulgardiner | yeah, that would make sense with what I see. | 09:41.30 |
| tor8 | paulgardiner: I was hoping it would catch /State changes without the hotspot flagging, but I see now I early out in pdf_update_appearance too soon | 09:41.43 |
| | give me a minute and I'll tweak that commit | 09:41.52 |
| | paulgardiner: take a look at f1040.pdf object 1600 (one of the check buttons) | 09:46.41 |
| | something looks weird to me | 09:46.45 |
| | http://ix.io/1fmT | 09:46.55 |
| | the AP has two appearances for AP/D but only one for AP/N | 09:47.12 |
| paulgardiner | That does sound weird | 09:48.22 |
| | I'd have expected the other way around | 09:48.36 |
| | But then again, mupdf looks to be displaying the correct one | 09:49.30 |
| | It's only the reporting of change that has problems, as far as I've noticed | 09:50.01 |
| tor8 | yeah, but why is it not detecting? let me prod around this a while, and I'll get back to you when I think I know some more | 09:50.32 |
| paulgardiner | Hang on, isn't it permissible to miss out the off state, meaning display nothing? | 09:50.58 |
| tor8 | or you can hunt too, in case we find it faster together | 09:51.00 |
| | paulgardiner: that's what I'm wondering, in which case the if (pdf_is_stream/dict) would fail | 09:51.15 |
| paulgardiner | Oh of course | 09:51.28 |
| | ... although I thought I traced that and it was returning the 0th element | 09:52.02 |
| tor8 | paulgardiner: yeah, that's it! | 09:52.32 |
| | and also what's causing us to gripe about not being able to create a Widget/Btn appearance stream | 09:52.57 |
| | paulgardiner: try tor/master top and penultimate commits | 09:55.58 |
| paulgardiner | Can I set to the end of the branch, or are you saying I should take those missing some out? I've been just using your branch up until now. | 09:58.00 |
| | He he! Now the unchecking one is not reported. | 10:04.32 |
| tor8 | paulgardiner: aacrrrgh! | 10:04.59 |
| | the top commit (WIP disable hotspot handling) should only remove toggling between D/N states (always show the annots in their normal, never depressed, state) | 10:05.47 |
| paulgardiner | So annot->ap is the current ap? Rather than the ap from last run of pdf_update_page? | 10:07.38 |
| | Not sure I understand the test at 1284 of pdf-appearance.c | 10:08.14 |
| tor8 | annot->ap is updated when you call pdf_update_page | 10:08.37 |
| | and when it is changed, it should also set the annot->has_new_ap to notify that it changed the value | 10:08.48 |
| | either when it changes because of the state, or it's recreated because the contents of the annot/widget changed | 10:09.33 |
| paulgardiner | Is it only within pdf_update_page that annot->ap gets update (other than initial loading of the annotation)? | 10:20.35 |
| tor8 | paulgardiner: so that is the ap from the last run of pdf_update_page, as you said, yes. | 10:20.41 |
| | and on the initial loading, we call that (so it sets has_new_ap) but the loading function resets has_new_ap to 0 (because it's the first time, so isn't actually changed) | 10:21.47 |
| | "we call pdf_update_annot" | 10:22.02 |
| paulgardiner | I wonder if it could be simplified by having pdf_update_appearance deal only with the needs_new_ap case and write the new ap into the document object hierarchy, but not set either annot->ap and annot->has_new_ap. And then leave the setting of those to the code that currently handles the hotspot. | 10:33.32 |
| tor8 | paulgardiner: I was considering moving the code that handles the hotspot moved into pdf_update_appearance (and then pdf_update_annot basically does nothing but delegate) | 10:40.27 |
| | the annotation creation updates the xobject AP in-place | 10:41.10 |
| | so we need to set the has_new_ap flag when doing that, since just looking at the pdf_obj won't tell us enough | 10:41.43 |
| inflex | evening tor8 . About to embark on trying to get this SDL version of muPDF running again. | 10:42.54 |
| tor8 | paulgardiner: how do you feel about having both ap_n and ap_d in the pdf_annot? | 10:43.01 |
| | instead of a single ap which toggles depending on the hotspot state | 10:44.06 |
| | that could let us simplify so that pdf_update_appearance (and pdf_update_annot) doesn't need to track up/down toggle state changes | 10:44.51 |
| | and leave that to the viewer | 10:44.56 |
| | and muso which doesn't bother with the down state would just ignore the ap_d and has_new_ap_d fields completely | 10:45.56 |
| paulgardiner | Not sure. Would certainly work well for muso, but then the viewer would still have to send in down and up events. Not sure if it seems odd to have the viewer sending in those events and still have to chose the ap | 10:52.55 |
| | The api where there is just the one seems simpler. I have no great objection though. | 10:53.33 |
| tor8 | paulgardiner: you still need the JS events to trigger, and it could stow a flag in the pdf_annot for what the current state is (so it doesn't need to query the pdf->hotspot.num and compare) | 10:53.47 |
| | and that's state as in norma/down/rollover not AS | 10:54.32 |
| | paulgardiner: it could help performance, if you render and cache each state separately, you wouldn't need to redraw when you mouse over or click | 10:55.05 |
| | something that would be difficult if there's just the one | 10:55.22 |
| | not that rendering an annotation is ever going to be very expensive all things considered | 10:55.56 |
| paulgardiner | But aren't I getting the performance improvement now, by just not calling pdf_update_page between down and up events? | 10:55.59 |
| tor8 | pdf_update_page doesn't do any work when just toggling states | 10:56.29 |
| | it only does any heavy lifting if it needs to recreate an AP from new content (i.e. when needs_new_ap is set) | 10:56.58 |
| paulgardiner | toggling states? You mean up/down? | 10:57.02 |
| tor8 | up/down and/or changing AS | 10:57.09 |
| | going from a checked to unchecked radiobutton is only a pointer switch of the annot->ap field | 10:57.48 |
| paulgardiner | Lost me a bit. I don't see where the performance improvement is coming from | 10:57.58 |
| tor8 | which is detected by has_new_ap, which triggers the viewer to rerender something | 10:58.17 |
| | if the viewer can save renders for both states, it wouldn't need to rerender anything when the state changes (only need to rerender for content changes) | 10:58.46 |
| paulgardiner | At the moment I avoid rerendering just for the sake of down up because I don't call pdf_update_page between the two so I don't see any annots with changed aps | 10:58.50 |
| tor8 | it would then just need to draw the other annotation render to screen when the state changes | 10:59.07 |
| paulgardiner | yeah, but isn't that already true. | 10:59.23 |
| tor8 | paulgardiner: think of the case of a viewer that has rendered the page to one texture, and each annotation to a separate texture, and does the final compositing to screen in the UI | 11:00.13 |
| | as it is now, whenever the state changes, it needs to recreate the texture for the annotation | 11:00.28 |
| | in the proposed change (separate ap_n and ap_d pointers), it would only switch which of the two textures are composited in the UI | 11:00.56 |
| paulgardiner | Oh I see. Got you. | 11:00.58 |
| tor8 | I'm not entirely convinced it's worth pursuing though, but it's an idea. | 11:01.53 |
| paulgardiner | yeah, not sure. I get the idea now at least. | 11:02.22 |
| tor8 | so, the code on tor/master is still broken (I can't uncheck an annotation once checked) | 11:03.34 |
| paulgardiner | Ah, what I suggested earlier wont work: if pdf_update_appearance recreates an ap, the actual pointer wont change, so the code at the bottom of pdf_update_annot wont detect the change. I guess that's what I was using the ap version number to detect | 11:04.19 |
| tor8 | paulgardiner: yeah, and I'm using the has_new_ap flag for the same effect as the old ap version number | 11:05.04 |
| | paulgardiner: bah, bad build. tor/master detects all changes (but doesn't show up/down on hotspots). does it not work for you? | 11:07.03 |
| paulgardiner | My last attempt didn't work. Maybe that was before your most recent change | 11:09.16 |
| | No. I'm on 52176284 | 11:11.08 |
| | I'll rm build, clean and rebuild | 11:12.11 |
| tor8 | paulgardiner: if I take out the pdf_update_page on 'down' events I also see that the connected radio box doesn't trigger | 11:13.16 |
| paulgardiner | okay good. That's what I see | 11:13.48 |
| | I'm trying to remember why I used version and not just a flag. I remember really not wanting to use version. | 11:14.29 |
| tor8 | I know why... | 11:15.19 |
| paulgardiner | Maybe I was trying to handle changes made via a different fzpage object. | 11:15.19 |
| tor8 | it fails tries and fails to create a new AP (because it was NULL for the blank state) and the catch case sets the has_new_ap to 0 in case of failure | 11:15.44 |
| | it's all down to that 'there is no AP for this state' case... | 11:16.01 |
| paulgardiner | Ah I see | 11:16.27 |
| tor8 | if I just leave it untouched (see the "argh!" commit on tor/master) it ought to work | 11:16.48 |
| paulgardiner | Just building | 11:18.30 |
| | Woo hoo! Whoop! Whoop! Wheeee! | 11:22.49 |
| | Works and so far not seeing any extraneous updates | 11:23.32 |
| tor8 | paulgardiner: I found one place where you could get extraneous updates | 11:25.37 |
| | loading the page again triggers a cascade of updates for me | 11:26.18 |
| paulgardiner | I see loads of "regular AP new or changed" reports, but I'm not detecting them as updates | 11:29.05 |
| tor8 | paulgardiner: yeah. but what's curious to me is I don't see them the first time the annot is loaded | 11:29.38 |
| | sorry, I was a bit unclear. | 11:29.46 |
| | those won't be extraneous updates, because pdf_load_annots nobbles the has_new_ap | 11:30.03 |
| | but I'm stumped as to why loading the same annot the first and second time would differ! | 11:30.17 |
| | the first time through AP/N/<as> is null, second time through it's set | 11:32.15 |
| paulgardiner | Might it be set by a javascript event? | 11:33.37 |
| tor8 | ah, it's all the blank text fields having new appearance streams created for them | 11:34.20 |
| | and that only triggers the first time, the second time through we'll find the old ones and that's causing us to print 'regular AP new or changed' | 11:35.03 |
| | annot->ap is always NULL when we load the annotation | 11:35.25 |
| | the first time around, there is no ap_n (since there is no AP for the text field) so that doesn't register as 'regular AP new or changed' | 11:35.57 |
| | but the second time, we've already created a text field and that triggers the printout | 11:36.11 |
| | so, false alarm, ignore me, everything is working as expected :) | 11:36.23 |
| paulgardiner | Magic. | 11:37.00 |
| tor8 | let me get this cleaned up | 11:37.18 |
| | paulgardiner: okay, top of tor/master should work now | 11:43.46 |
| | for the checkboxes | 11:43.57 |
| paulgardiner | Will give it a go | 11:44.02 |
| tor8 | paulgardiner: bah, I know why it looks like the CHOICE-ausfell thing choices are all overwritten | 11:51.44 |
| | try deleting the choice widget... hidden behind the background color is the text | 11:52.10 |
| paulgardiner | The cleaned up version works fine. | 11:55.44 |
| tor8 | paulgardiner: I've got fixes for (I hope) all of the issues that you emailed me files for on tor/master | 12:31.09 |
| paulgardiner | Great. Will give them a try | 12:31.36 |
| moolc | tor8: nostar is topic branch for pass by value things? | 16:58.40 |
| sebras | moolc: yes, but tor8 and robin seem to still be undecided. tor8 is measuring performance. | 17:14.04 |
| moolc | sebras: äºäº | 17:15.01 |
| | sebras: http://connect.linaro.org/resource/hkg18/hkg18-403/ ... are they trying to steal lauterbachs thunder? | 17:17.17 |
| | Forward 1 day (to 2018/06/30)>>> | |
Log of #mupdf at irc.freenode.net.