feature: Implement reading mask data #56
Conversation
|
We are definitely interested. Keep up the good work! |
turns out in certain cases given length is much more than real length of a channel - and we need to read scanline sizes in order to calulcate that. Also, in case of masks, number of scanlines is proportional to mask height, not layer height.
scoiatael
force-pushed
the
feature/mask-data
branch
from
fb13da7
to
251cc51
Compare
Originally from here: webtoon@9cd2d8f NOTE: I cannot reproduce it on my sample files, and there's no test file. So I can only hope to preserve compatibility :)
| @@ -16,15 +16,15 @@ export class Group implements NodeBase<NodeParent, NodeChild> { | |||
|
|
|||
| /** @internal */ | |||
| constructor( | |||
| private layerFrame: GroupFrame, | |||
| private layerFrame: GroupFrame | undefined, | |||
There was a problem hiding this comment.
This is very weird to me, but there is one sample file where rendering layer fails here with:
TypeError: Cannot read properties of undefined (reading 'layerProperties')
at get opacity [as opacity] (file://./packages/psd/dist/index.js:1:186)
at get composedOpacity [as composedOpacity] (file://./packages/psd/dist/index.js:1:273)
at get composedOpacity [as composedOpacity] (file://./packages/psd/dist/index.js:1:78558)
at BI.composite (file://./packages/psd/dist/index.js:1:78007)
at async file://./packages/psd/scripts/parse.mjs:113:7
at async Promise.all (index 256)
at async file://./packages/psd/scripts/parse.mjs:109:3
where parse.mjs is
import * as fs from "fs";
import Psd from "../dist/index.js";
for (const input of process.argv.slice(2)) {
console.log(input);
const arrayBuffer = fs.readFileSync(input);
const psd = Psd.parse(arrayBuffer.buffer);
await Promise.all(
psd.layers.map(async (layer) => {
await layer.userMask();
await layer.realUserMask();
await layer.composite();
})
);
}thus the workaround.
|
Tested mask extraction on sample files - resulting images look legit. Ready for review :) Do you think we should add a test to ensure that rendered user mask image is ok? This'd require most likely simply having rendered png in repository and comparing against that, as I don't see any other way of testing it. |
I don't think we need an actual PNG file. It would make managing tests easier, but we'd have to decode the PNG file when running the test to compare the raw image data. Alternatively, you could dump the expected return value of |
| // Skip the Layer Mask info segment, which we don't need for now | ||
| // Read the length of the segment and skip it |
There was a problem hiding this comment.
We can happily delete this comment :)
This is easier than having full binary dump on disk and just as useful ;)
Good idea! Implemented it using |
|
IMO computing a hash is less preferable to decompressing a PNG in memory, because a test failure doesn't tell you anything. At least with a compressed stream, we can compare the contents to see how and where the code failed. If the size of the bin is a concern, we could double back on the PNG idea; or simply GZIP the bin file using |
|
Yeah, the question is how far we wanna go to make future debugging easier. In my experience when it comes to problems with binary files it's rarely worth extra hassle, because "mismatch at offset 0xdeadbeef" gives you precisely as much information as "hash mismatch" - you'll need to dump the file and look for yourself anyway :) We are already guarded against the easy case of file truncation by having assertion on resulting data size - maybe this, plus hash check if enough? |
|
Good point on the OOM problem. I remember using weird tricks to prevent test failure messages from spilling all over the terminal. I wish there was a test reporter that can concisely summarize binary diffs. But in the meantime, your approach is completely valid. 👍 Feel free to merge this PR. I'm waiting for the other PRs as well. P. S. I'm planning a new release soon--as early as this Friday. Do you want to have all your PRs in the new release? If not, I can arrange your PRs to be included in another release (hopefully also published before 2023). |
|
Only you can merge the PR, I don't have write rights to your repo :) As for the releases - don't worry, in current setup we don't need them, as we have this repo in git submodule which we build on our own :) |
|
My bad, I am not familiar with GitHub roles yet :P I gave you Write access so you can merge PRs directly. Thanks for your ongoing contributions! |
* [feature] implement reading mask data * bugfix: decoding mask channels turns out in certain cases given length is much more than real length of a channel - and we need to read scanline sizes in order to calulcate that. Also, in case of masks, number of scanlines is proportional to mask height, not layer height. * bugfix: group frame may be undefined * bring back previous bugfix (f6bf97c) Originally from here: webtoon@f6bf97c NOTE: I cannot reproduce it on my sample files, and there's no test file. So I can only hope to preserve compatibility :) * Remove obsolete comment * Describe what masks should look like, assert hash This is easier than having full binary dump on disk and just as useful ;) * Use `height` helper also for layerRecord * Fix test name
Hey,
Over the past month we implemented some features in our fork for the upcoming OpenDesign new release. We didn't have time to discuss them with you properly, so let me know if you want these features or not - in case you do, I'll properly refactor the code and add tests :)
This is PR 3/3; implementing layer mask properties and decoding.
mask.psdcomes frompsd-toolstests (https://github.com/psd-tools/psd-tools/blob/main/tests/psd_files/masks.psd): Python package, released under MIT license.