| | <<<Back 1 day (to 2016/08/16) | 20160817 |
| sebras | Robin_Watts: tor8 (for the logs): there is a small patch over at sebras/master the resolves the broken_png_image.xps indeterminism. | 03:01.35 |
| | basically we assumed we could trust entry sizes in the archive's central directory but we really can't. | 03:02.09 |
| | addressing that quiets valgrind and supposedly the indeterminism. | 03:02.35 |
| Robin_Watts | sebras: Looks good to me. Might be nice to mention in the commit message that this fixes that particular file. | 09:53.04 |
| sebras | Robin_Watts: luckily that can be fix easily. | 11:12.59 |
| | Robin_Watts: commit message update and commit pushed to golden. | 14:43.51 |
| Robin_Watts | sebras: Cool. | 14:44.03 |
| sebras | Robin_Watts: tyfr! | 14:44.32 |
| | Robin_Watts: tor8: draw-, svg- and trace-device derefs NULL if stroke == NULL. I take that as indication that I should throw a JNI exception. | 14:59.39 |
| Robin_Watts | sebras: Yes, it makes no sense to stroke a path without a stroke state telling you how to do it. | 15:02.42 |
| sebras | Robin_Watts: I'll do the same for clipping. | 15:02.58 |
| | and likely for text fill/stroke without text. | 15:03.12 |
| | will handle cs == NULL as usual in the latter case though. | 15:03.21 |
| | Robin_Watts: btw, does it matter if I throw IllegalArgumentException or NullPointerException? | 15:03.49 |
| | to me the former seems more helpful. | 15:03.57 |
| Robin_Watts | NullPointerException = "shit, I dereferenced a NULL". | 15:05.02 |
| | i.e. it's a "something went wrong with the lib you are calling". | 15:05.14 |
| | IllegalArgumentException = "You're calling us wrong!" | 15:05.29 |
| | so I agree, that one is better. | 15:05.35 |
| sebras | have a look at java.lang.Character.codePointCount() if the sequence argument is null it will throw NullPointerException | 15:05.57 |
| | String.join() al;so throws NullPointerException on a null delimiter argument. | 15:07.02 |
| | hard to know what is right here. I'll stick with IllegalArgumentException still though. | 15:07.29 |
| | Robin_Watts: fz_draw_begin_mask() with colorspace == NULL...? | 16:58.20 |
| | Robin_Watts: seems like if !luminosity we generate a blank pixmap | 16:58.59 |
| | Robin_Watts: and if luminosity we default the colorspace to gray. | 16:59.10 |
| | but the comment says we should tak e the alpha value from the shape if !luminosity. | 16:59.39 |
| Robin_Watts | sebras: What comment? | 17:01.23 |
| sebras | If !luminosity, then we generate a mask from the alpha value of the shapes. | 17:01.42 |
| | If luminosity, then we generate a mask from the greyscale value of the shapes. | 17:01.47 |
| Robin_Watts | If luminosity, then we want to draw the shapes as normal, and at the end of the process, we convert it to greyscale. | 17:02.00 |
| | So we just create a greyscale bitmap to start with, and whenever we draw into that, stuff will get converted to greyscale anyway, and we'll have what we need. | 17:02.34 |
| | The sole point of the colorspace/color in that case is because it gives the background color. | 17:03.23 |
| | i.e. the color that we blank the pixmap too. | 17:03.33 |
| sebras | Robin_Watts: right, but why is it safe to assume fz_device_gray() if colorspace == NULL? | 17:03.57 |
| Robin_Watts | sebras: Just looking at that. | 17:04.15 |
| sebras | I think that might be wrong. | 17:04.22 |
| | but all of these things are quite new to me (hence the many questions and me being tentative about many steps) | 17:05.01 |
| | Robin_Watts: to me it semms colorspace controls how many components are assumed to be in colorfv. | 17:05.34 |
| Robin_Watts | I suspect that if colorspace == NULL, then we shouldn't be calling fz_convert_color, hence we wouldn't need to set colorspace. | 17:05.40 |
| | Yes, exactly. | 17:05.42 |
| sebras | Robin_Watts: and if colorspace == NULL then there is only a single component in colorfv. | 17:05.49 |
| Robin_Watts | but if colorspace == NULL, what color should we clear to? | 17:06.07 |
| sebras | Robin_Watts: exactly. is this a case where colorspace == NULL does not mak3e sense4? | 17:06.23 |
| Robin_Watts | possibly if we're assuming a single component in the colorspace = NULL case, then using fz_convert_color with greyscale will give us the right thing. | 17:06.37 |
| | sebras: I'd be tempted to add some if (colorspace == NULL) crash() code there and clusterpush. | 17:07.00 |
| sebras | done. | 17:07.45 |
| Robin_Watts | and if that comes back clean, then make it throw on being given a NULL colorspace with luminosity being true. | 17:07.51 |
| sebras | Robin_Watts: right, but we could do that a lot earlier before even creating the pixmap... | 17:09.16 |
| Robin_Watts | sebras: Right, assuming there is not some case here that we aren't seeing. | 17:09.40 |
| sebras | Robin_Watts: I should be able to test this inside the first if(luminosity) since luminosity doesn't change inside the function. | 17:10.40 |
| Robin_Watts | sebras: indeed, yes. | 17:11.38 |
| sebras | Robin_Watts: what happens when DrawDevice_newNative() returns 0? | 17:12.37 |
| | does the JNI-framework do something in this case? | 17:12.47 |
| Robin_Watts | public class Device() { pointer = newNative(); } | 17:14.54 |
| | so no. What it *should* do is throw an exception if pointer comes back as 0. | 17:15.07 |
| | or we could raise the exception in the JNI layers. | 17:15.29 |
| | the latter is probably better. | 17:15.54 |
| sebras | Robin_Watts: well I'm throwing IllegalArgumentExceptions in the other cases. | 17:16.00 |
| | agreed. | 17:16.15 |
| | Robin_Watts: 6 files segfaults when I replace fz_device_gray() with abort() in fz_draw_begin_mask()... | 17:17.08 |
| | so throwing in that case might not be viable. | 17:19.10 |
| Robin_Watts | So you have 6 example files to show you where our thinking has gone wrong. Or to take you down another layer of the rabbit hole. | 17:19.18 |
| sebras | given that one is called xobject messing up clip stack I'm _really_ tempted to look at these. ;) | 17:20.10 |
| Robin_Watts | sebras: Oh, it's 1 source file, in 6 different tests ? | 17:21.47 |
| sebras | Robin_Watts: no six files. | 17:22.00 |
| | Robin_Watts: e.g. the one from bug 693681, two from FTS and the rest from sumatra | 17:22.33 |
| Robin_Watts | sebras: The FTS ones are likely to at least be well formed. | 17:31.22 |
| sebras | Robin_Watts: seems to be that I didn't handle cs == NULL in list-device.c in a couple of places. | 17:33.04 |
| | so I iterate the color array in the code even when I shouldn't be doing that. | 17:33.27 |
| | Robin_Watts: do you mind helping me out a little bit more? | 17:54.58 |
| Robin_Watts | sure. | 17:55.04 |
| | Forward 1 day (to 2016/08/18)>>> | |
IRC Logs