mozilla :: #pdfjs

12 Jul 2017
12:09duziIn colorspace.js, there is a method Colorspace.getRgbItem which just throws an error. Is it just a stub? If yes, should I avoid writing tests for it?
13:43yuryduzi: it's abstract class. you can skip tests for it
13:44duziOk.
13:45duziyuri: Can you elaborate what 'IR' is(in that file)?
13:46duziWell, I know it can be an array or a string. Just wanted to know what does the name signify?
13:51yuryduzi: it stands for Intermediate Representation
13:51duziOk.
13:51yuryin PDF.js context, it is something which is send between worker and main threads
13:52yuryhttps://en.wikipedia.org/wiki/Intermediate_representation
13:57duziOk. Also, one more thing. In which case does the method Colorspace.parse(...) return and IR which is an instance of AlternateCS?
13:57duzi*an
14:10yuryhttps://github.com/mozilla/pdf.js/blob/7b4887dd211c44d5ef852ad019921fb41fa3709b/src/core/colorspace.js#L357
14:10yuryduzi: Separation and DeviceN are AlternateCS
14:17duziyury: That part returns an array representing AlternateCS. My question is regarding https://github.com/mozilla/pdf.js/blob/7b4887dd211c44d5ef852ad019921fb41fa3709b/src/core/colorspace.js#L207. How would this condition be true when we are always returning an array or string. We are never returning an instanceof AlternateCS afaik. I am not sure about case 'ICCBased' though.
14:19yurywe were moving processing from main thread to worker, so there might be dead paths
14:19yuryor inefficient code
14:19yuryduzi: more you can cover in spec -- better
14:20duziOk.
14:20yurysee also https://github.com/mozilla/pdf.js/issues/8632
18:36Pomaxhttps://github.com/mozilla/pdf.js/pull/8642
18:36Pomaxyury: small PR due to security announcements by Nodejs
18:39yuryPomax: thanks, there is an extra comma at line 38
18:39Pomaxremoved spurious comma
18:40yurycan you squash that?
18:40* yury wonders if there is a button in guthub.com for that
18:40Pomaxyou can.
18:40Pomaxmerge button has a pulldown
18:40Pomaxchange it to "squash and merge"
18:41Pomaxsoooo handy
18:41yurywe usually don't do that :)
18:41Pomaxif it can't clearly squash, it'll even let you do conflict resolution on line now
18:42Pomaxwhich I'll be honest, beats the hell out of a text editor and seven cli git commands
18:43yuryokay
18:50mukulyury: can you see this: https://github.com/mozilla/pdf.js/pull/8617#discussion_r127033348
18:51mukulI think we need `ReaderDocProgress` message too.
18:51yurymukul: why did you remove code from ReaderHeadersReady?
18:52yuryinstead of sending a message it could just can do whatever DocProgress is doing
18:53mukulSo you mean we can directly do `if (loadingTask.onProgress) {...` in fullReader.headersReady.then()
18:54mukulBut I can't understand why it is breaking progress bar
18:54yurysure, but you can make one step further and extract `if (loadingTask.onProgress) {...}` into some method
18:56yurysomething wrong with the DocProgress -- its logic is not the same
18:56yurytry to keep logic intact
18:58mukulearlier it is sending `DocProgress` back to main only when fullRequest.onReaderDocProgress is set.
19:07yurymukul: DocProgress is also send from chunked stream
19:07yurythis logic must be preserved
19:09mukulyury: okay, but i tested viewer locally and I can't find anything weird.
19:10yurynotice that onReaderDocProgress is bound when range (and streaming) is disabled
19:10mukulI think I am doing something wrong.
19:11yurytry to revert onReaderDocProgress change until it works, make small steps
19:16mukulcan you please tell me how to test for progress bar, I think I am misunderstanding this. As I don't see any weired behaviour in viewer.
19:27yurytry with #disableRange=true and/or #disableStream=true
19:28yurymukul: https://github.com/mozilla/pdf.js/wiki/Debugging-PDF.js#url-parameters
19:37duziWhat does the src and srcOffset params in https://github.com/mozilla/pdf.js/blob/7b4887dd211c44d5ef852ad019921fb41fa3709b/src/core/colorspace.js#L69 signify?
19:38yuryduzi: there is a comment above, src is usually Uint8Array and srcOffset pointer to the byte where data starts
19:40mukulyury: Okay, I think it is working now. Calling `loadingTask.onProgress` directly in `ReaderHeadersReady` make it work.
19:42duziyury: What could be the range of the size of src array?
19:46yuryduzi: the getRgb consumes exactly the number of needed components
19:46yurysometimes it's one, two, three or four
19:47yuryduzi: notice numComps property is different on every CS
19:48duziso you mean src.length === numComps?
19:48yurysrc.length >= srcOffset + numComps
19:49yuryfor getRgb it should not matter
19:53duziWhat does the src array contain? Is it the data related to colors only or does it contain other data too. I guess if it contains other data, then the data regarding colors starts at srcOffset. Correct me If I am wrong.
19:55yuryduzi: so colorspace classes convert the same color from one color space to another
19:56yurye.g. cmyk color 10,20,30,0 will be converted to some rgb color with three component
19:57yuryevery color is represented by more than one number and mutilple colors are usually packed in one continuos array
19:57yuryduzi: make sense so far?
19:58duziOk
19:59yuryso while for cmyk 12 number array contains 3 colors, it contains 12 colors for gray cs
20:01duziyury: Sorry, I don't get this statement. Can you please elaborate a bit?
20:03yuryif src is array of [0,1,2,3,4,5,6,7,8,9,10,11], there 3 cmyk colors (0,1,2,3),(4,5,6,7),(8,9,10,11)
20:03yurythere are
20:04yurysrcOffset can be 0, 4, 8 for getRgb
20:05duziOk
20:11duziWhy do we multiply by 255 in https://github.com/mozilla/pdf.js/blob/7b4887dd211c44d5ef852ad019921fb41fa3709b/src/core/colorspace.js#L583
20:12yurysorry, I forgot it is Float32Array, so components in range from [0 .. 1.0)
20:13duzisrc array right?
20:13yuryyeah
20:13yurythey will become Uint8Array with components in range [0..256)
20:13duzialright, so you basically multiply by 255 to scale up the value to come in range [0, 255], right?
20:14yuryif it's rgb
20:15duziyeah, depends on the bits each component holds, I guess.
20:17bdahlyury: on https://github.com/mozilla/pdf.js/pull/8627/files#diff-9f8bcb636ae5be6a83003cdc08420bacR224 does using the default nominalWidthX/defaultWidthX not work?
20:28yurybdahl: nope
20:29bdahlyury: what file does it not work?
20:29yurytaro
20:29yurysvg
20:29yurywe don't support vertical orientation, but you will see the defect
21:00duziyury: I see only ColorSpace exported in https://github.com/mozilla/pdf.js/blob/7b4887dd211c44d5ef852ad019921fb41fa3709b/src/core/colorspace.js#L1294. Should I also export various color space families since I need to test them?
21:02yuryduzi: can we use ColorSpace.parse to reach them?
21:08duziI don't think so. Even if ColorSpace.parse(...) returns the appropriate CS, how would I be able to access its properties? Correct me if I'm wrong?
21:09yuryduzi: you will need only check if such function getRgb return right values
21:09yurynot need to inspect internal properties of specific CS
21:11yuryduzi: we are interested only what ColorSpace exposes, e.g. getRgb, getRgbBuffer, getOutputLength, and maybe isPassthrough and numComps
21:11duzibut getRgb in turn calls getRgbItem whose implementation differs for various CS I guess
21:12yurygetRgb calls getRgbItem
21:12yuryyou can test getRgbItem instead of getRgb
21:13duziWe would have to test getRgbItem for each specific CS, right?
21:13yuryyes
21:14duziThen, we would need to export the respective CS, right?
21:14yurywhy? an CS is built via ColorSpace.parse
21:15yuryduzi: notice that any CS is inherited from ColorSpace
21:17duziSo, you mean we simple write ColorSpace.parse(...).getRgbItem?
21:17duziduzi: Would that work?
21:18duzi*simply
21:19yuryvar cs = ColorSpace.parse( some_dict_xref_res_data_to_init_right_cs);
21:20yurycs.getRgb(..)... cs.getRgbBuffer(...)... cs.getOutputLength(...)...
21:20yurywe have some _spec.js to init fake xref and res
13 Jul 2017
No messages
   
Last message: 74 days and 16 hours ago