Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PSD export example #239

Merged
merged 8 commits into from
Apr 17, 2023
Merged

Add PSD export example #239

merged 8 commits into from
Apr 17, 2023

Conversation

andersmelander
Copy link
Member

The code needs to be cleaned up a bit before it can be merged.

I'm also getting the occasional range check error or overflow error when saving PSD.

Initial tests show that some applications have problems reading the generated PSD while others read it fine. This could very well be a problem with the readers and not in the generated files.

lamdalili and others added 2 commits April 15, 2023 13:01
@andersmelander andersmelander self-assigned this Apr 15, 2023
@andersmelander andersmelander linked an issue Apr 15, 2023 that may be closed by this pull request
@andersmelander
Copy link
Member Author

@lamdalili I can see that the original code (in your repo) is GPL licensed. Graphics32 uses the MPL 1.1 or LGPL 2.1 licenses.
Are you okay with your code being included under the Graphics32 license (i.e. not GPL)?

Anders Melander added 2 commits April 16, 2023 02:46
@lamdalili
Copy link
Contributor

I'm also getting the occasional range check error or overflow error when saving PSD.

I wrote it with the default configuration, if range checking is enabled, errors may occur.

Initial tests show that some applications have problems reading the generated PSD while others read it fine. This could very well be a problem with the readers and not in the generated files.

I think this due to the zip compression which isn't supported by all editors,
in the example change Psd.PsdLayerCompression := psComZip; to psComRLE or save without any comprsesion and see if the error remains,

Are you okay with your code being included under the Graphics32 license (i.e. not GPL)?

yes, no problem for that.

@lamdalili
Copy link
Contributor

lamdalili commented Apr 16, 2023

I found some range erreurs :
Out8(R - 1); here

Also the call to Swap needs to be protected when the var is signed SmallInt(Swap ()) inverting bytes order may produce invalid value for this type

More strict type declarations; Fixed remaining range check errors.
Added UI for selection of compression.
Refs #239
@andersmelander
Copy link
Member Author

I think this due to the zip compression which isn't supported by all editors,

Yes, I suspected that much but I think there's more to it.
For example, this online viewer has problems with the ZIP compressions: https://www.fviewer.com/view-psd
It loads them but displays junk in the layers.

While this one handles ZIP but not ZIPrd and complains about an invalid signature with all compressions: https://www.photopea.com/

The PSD reader in GraphicEx refuses to load anything regardless of compression. I have previously used it to load PSD so I know it works.

Anyhow, I believe I have now resolved all the issues regarding range check errors.

@andersmelander
Copy link
Member Author

The PSD reader in GraphicEx refuses to load anything regardless of compression.

It was a bug in GraphicEx; It didn't take alignment padding between adjustment layers (e.g. Unicode layer name) into account. Now it works:

billede

I can see that the GraphicEx PSD reader only supports RAW and RLE compression, so it will need some minor work to be able to support full round-tripping.

@andersmelander andersmelander marked this pull request as ready for review April 16, 2023 21:59
Copy link
Member Author

@andersmelander andersmelander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell we're ready to merge.

@lamdalili
Copy link
Contributor

Great ,I see a lot of progress in the project and a lot of bugs been fixed,
it's a real challenge to make it compatible for all editors. I think it's still doable if opening the file in Photoshop and save new copy and then check the ajusted or added parts this gives idea about default configuration.

While this one handles ZIP but not ZIPrd ..

Actuelly they use the same function, the code for the compression operation was improvised and needs to be rebuilt.

@andersmelander
Copy link
Member Author

While this one handles ZIP but not ZIPrd ..

Actuelly they use the same function, the code for the compression operation was improvised and needs to be rebuilt.

Ah! So it probably does handle ZIPrd but we're not supplying the correct data.
Then I think it would be better to disable that compression type for now.

@lamdalili
Copy link
Contributor

Ah! So it probably does handle ZIPrd but we're not supplying the correct data.

Yes, i dont no idea what's the difference and how it should be implmented.

Can you laod the image in the example as first layer this makes test more random instead of those monolitic shapes with few colors.

@andersmelander
Copy link
Member Author

Yes, i dont no idea what's the difference and how it should be implmented.

Okay, no problem. It can wait for later. This is a great start regardless.

I had a quick look at how it's done in ImageMagick but I don't really have time to do anything with it right now. I also think it has low priority compared to also being able to read PSD files.

@andersmelander andersmelander merged commit 2aee194 into master Apr 17, 2023
@lamdalili
Copy link
Contributor

lamdalili commented Apr 17, 2023

Another problem, image "or what we call background here" for most editors supports only the RLE compression, that's why I set it permanently in the code, L738, for now Compression has no effect `WriteSwappedWord(FStream, Ord(psComRLE));

const
  FolderMedia = '..\..\..\..\..\Media';

The last commit for loading static jpg doesn't work I 've error "file not found" or some thing like; currently the build output folder isn't configured, and the build is at the same level of poject.

@andersmelander
Copy link
Member Author

Another problem, image "or what we call background here" for most editors supports only the RLE compression, that's why I set it permanently in the code,

Yes, I noticed that. I think it's sensible to have RLE as the default, but since it's possible to specify another compression we really should respect that and use the compression specified.

Anyway, I have begun work on moving your PSD-writer into an image format adapter. I will push some code (into a new branch) for it later today.

The last commit for loading static jpg doesn't work I 've error "file not found" or some thing like; currently the build output folder isn't configured, and the build is at the same level of poject.

That's strange. It should be correctly configured. This is what I'm seeing:

billede

What version of Delphi are you using?

@andersmelander
Copy link
Member Author

That's strange. It should be correctly configured. This is what I'm seeing:

Ah! I've forgotten to push the project settings file. I'll get that committed now.

@lamdalili
Copy link
Contributor

Ah! I've forgotten to push the project settings file. I'll get that committed now.

Thanks it works now.

@lamdalili
Copy link
Contributor

It was a bug in GraphicEx; It didn't take alignment padding between adjustment layers (e.g. Unicode layer name) into account. Now it works:

Maybe, but for Photopea, the problem persists if you append the name by one character, an error message occurs, the problem is when you have two variable aligned sections, you don't know at which one the error happened, by adding null character you just aligned ExtraSection by 4 (the spec accepts alignment by 2 but applications use alignment by 4 ) )

This value should be updated to 4

@andersmelander
Copy link
Member Author

This value should be updated to 4

Please create a pull request and I'll merge it.

@andersmelander
Copy link
Member Author

Maybe, but for Photopea, the problem persists if you append the name by one character, an error message occurs, the problem is when you have two variable aligned sections, you don't know at which one the error happened, by adding null character you just aligned ExtraSection by 4 (the spec accepts alignment by 2 but applications use alignment by 4 ) )

I'm still seeing the same Photopea warning with ZIP-compressed PSDs.

@lamdalili
Copy link
Contributor

lamdalili commented Apr 24, 2023

Yes I see the error before and I try to understand it, the error happens when Photopea tries to read Layer Glabal Mask which is located just after the LayerInfo section containing the image data, The LayerInfo section is aligned to 2 as shown in the spec, but again needs to be moved to 4 as in the previous case.

spec:

Length of the layers info section, rounded up to a multiple of 2.
https://www.adobe.com/devnet-apps/photoshop/fileformatashtml/#50577409_16000

@andersmelander
Copy link
Member Author

andersmelander commented Apr 24, 2023

The LayerInfo section is aligned to 2 as shown in the spec, but again needs to be moved to 4 as in the previous case.

Okay. I'm trying that out now... Yes, that did the trick. I'll commit the change.

While comparing the spec to the implementation I noticed this in the description of the Channel image data:

If the layer's size, and therefore the data, is odd, a pad byte will be inserted at the end of the row.

I can't see that we're doing that.

andersmelander pushed a commit that referenced this pull request Apr 24, 2023
@lamdalili
Copy link
Contributor

I can't see that we're doing that.

I'm not sure understand all, if talking about each channel separately this might cause problem for decoders, otherwise all other sections at this scope are aligned and any misaligned image data will be corrected, if you know how to handle it you can update the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

question : Saving to PSD ?
2 participants