thenickdude / chickenpaint

An HTML5 Port of the ChibiPaint multi-layer Oekaki painting tool
GNU General Public License v3.0
120 stars 21 forks source link

feature: ability to load images into active layer / save and load layered data #28

Closed blurymind closed 3 years ago

blurymind commented 3 years ago

Chickenpaint appears to lack the ability to load image data into a layer. You cannot paste/load an image from clipboard or from disk.

There is no way to export the layered image either - one can only getFlatPNG().

So while we can paint and save images, we can never actually continue from before we left off. This kind of completely cripples the app for any painting activity that is not going to be a single session.

thenickdude commented 3 years ago

You can load .chi files by supplying the loadChibiFileUrl option though? Saving and loading layers is fully supported.

Paste from external apps is intentionally not supported so that all the work on our Oekaki forum is made using ChickenPaint only. This means people can't "cheat" and use a more powerful, expensive, desktop app.

blurymind commented 3 years ago

Is it possible to load and save a chi file without using a url? For my case I am not using the app as a part of an oekaki board, but as embedded in a react app.

On Tue, 18 May 2021, 20:20 Nicholas Sherlock, @.***> wrote:

Closed #28 https://github.com/thenickdude/chickenpaint/issues/28.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thenickdude/chickenpaint/issues/28#event-4762569943, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVI53MOCC2LJKBSD7IDTOK4XFANCNFSM45DB6HXQ .

thenickdude commented 3 years ago

Yes, insert your own modified version of CPResourceLoader and CPResourceSaver to implement your persistence layer.

blurymind commented 3 years ago

Yes, insert your own modified version of CPResourceLoader and CPResourceSaver to implement your persistence layer.

can that be exported from chickenpaint's module? How do you insert it?

Btw if you are curious, I have set up an experiment react project using chickenpaint here https://github.com/blurymind/react-pwa-boilerplate

for now its just a collection of stuff, but I might grow it out to be a mobile friendly pwa for creating rough visual novels. Chickenpaint would let the user draw a room/avatar directly in it.

I got it to store its flattened image to the cache, which is what the visual novel engine uses to load resources - but that stuff is not yet complete. The code is a bit of a mess, but hey chickenpaint loads and can save to cache

Chickenpaint now being on npm, dont you think its API should be more user friendly to encourage usages outside of bbcode boards without a lot of extra work?

blurymind commented 3 years ago

Here is an online demo https://blurymind.github.io/react-pwa-boilerplate/#/paint

if you save the image with the save button,it will appear in the files tab and the debugger will show it in cache

I am not sure how to do what you are suggesting. Wouldnt it be simpler to just expose the api with a method to set/get the stuff rather than hackily override its classes? It doesnt seem like a good solution to me.

If you want people to use, wouldnt it be easier for others to have such methods exposed? Are you open to a PR that add them? I can do that, but if you are going to reject it I dont want to waste my time creating it. Might still use it just for myself on a fork

thenickdude commented 3 years ago

The thing about building an API is that a design built for the needs of only one caller is basically always wrong. It isn't until you have 2 or 3 different implementers that you can figure out what a general purpose API should look like for that task that covers everyone's needs. ChickenPaint's resource savers are built around an HTTP API that was already in common use for multiple other Oekaki applets.

So please fork it and maintain a fork, I don't think it's worth adding it upstream until it's clear that there are other people who can also use that exact implementation.

blurymind commented 3 years ago

In my use case, I found that getFlatPNG kind of gives me what I needed for getting the image. Woudlnt it be nice to have similar methods for getting the layered file and setting it?

The thing is that probably a lot of projects that need a painting component will not be oekaki boards. If you are dead set on keeping its usage narrow to oekaki boards then it does seem right to create a fork project that does more than that.

In the end it is just a couple of methods that are optional

On Wed, May 19, 2021 at 1:17 PM Nicholas Sherlock @.***> wrote:

The thing about building an API is that a design built for the needs of only one caller is basically always wrong. It isn't until you have 2 or 3 different implementers that you can figure out what a general purpose API should look like for that task that covers everyone's needs. ChickenPaint's resource savers are built around an HTTP API that were already common for other Oekaki applets.

So please fork it and maintain a fork, I don't think it's worth adding it upstream until it's clear that there are other people who can also use that exact implementation.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thenickdude/chickenpaint/issues/28#issuecomment-844047675, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVL7PM5BVXRTOOBLIYTTOOT57ANCNFSM45DB6HXQ .

thenickdude commented 3 years ago

Import the load and save functions from CPChibiFile.js and give them the artwork.

You can't directly load a new artwork into the editor after init though, because it keeps too much state and event handlers from the old one, and there is no destroy/restart implementation.

thenickdude commented 3 years ago

Also if you're using Babel/a bundler tool in your project already, you should be able to import chickenpaint/js/ChickenPaint.js directly instead of the bundled version you get when doing require("chickenpaint").

blurymind commented 3 years ago

thanks I will give this a try in my experimental react project before deciding to fork :)

blurymind commented 3 years ago

CPChibiFile.js

How do I give them the artwork though - pass it to what in chickenpaint's instance? I need to first figure out how to get it from chickenpaint as a file that contains layers, which I have no idea how to do.

I can get it as a flat png, but that loses the layers. There is no method to get it layered.

thenickdude commented 3 years ago

Aren't you calling getFlatPNG on the artwork already? That's it. Pass that in

blurymind commented 3 years ago

so there is no way of exporting it with layers? I was hoping of getting the .chi file, you mentioned :)

thenickdude commented 3 years ago

??? Take the artwork you already have a reference to and pass it to the save function from CPChibiFile.js

blurymind commented 3 years ago

You can load .chi files by supplying the loadChibiFileUrl option though? Saving and loading layers is fully supported.

You said this earlier. So I assume that if I have a chi file in my cache and pass its url to chickenpaint when initializing, that will load it?

The question is how do I get the chi file out of chickenpaint and into the cache - instead of the flat png.

I can already load stuff into the cache - I just dont know how to get the chi out of chickenpaint

blurymind commented 3 years ago

so when we have this:

new ChickenPaint({
    uiElem: document.getElementById("chickenpaint-parent"),
    saveUrl: "save.php",
    postUrl: "complete.php",
    exitUrl: "index.php",
    resourcesRoot: "chickenpaint/"
});

Lets imagine that instead of php we have an ordinary pwa app, hosted on github.io, which btw doesnt allow POST requests. How do we do it? :)

thenickdude commented 3 years ago

I don't understand where this confusion is coming from. This is your current code:

const ChickenPaint = require("chickenpaint");
const flat = binaryStringToByteArray(            
  chickRef.current?.getArtwork().getFlatPNG();
);

Use:

const ChickenPaint = require("chickenpaint");
const {save} = require("chickenpaint/js/engine/CPChibiFile.js");
save(chickRef.current?.getArtwork()).then(chibiResult => {
   console.log(chibiResult);
});

You'll want chibiResult.bytes out of that result

blurymind commented 3 years ago

thanks, I will try this tonight :+1: :)

blurymind commented 3 years ago

I get this image

blurymind commented 3 years ago

I have commited it btw,so you can see

blurymind commented 3 years ago

layer does exist, but there is no layers object attached to it image

curiously the layers are in the parent Object

blurymind commented 3 years ago

const isImageLayer = layer instanceof CPImageLayer, it looks like it fails because it thinks its not an imageLayer, which triggers the thing that crashes it :(

how very odd, here is the console log, right before it image

blurymind commented 3 years ago

If you do

    const
        isImageLayer = layer.constructor.name === "CPImageLayer",

that fixes it, but why does it not work as it is? instanceof should do it too, but it doesnt. typeof layer yields "object"

Should I make a pr for this?

blurymind commented 3 years ago

I got to the next blocking problem. This option here: //loadChibiFileUrl: "./uploaded.chi",

I tried passing to it the datauri of the cached chi file and it seems to not work. How would one load a chi file when it is in the browser's cache instead of hosted on a server?

Btw the datauri method works for me when using it with monogotari (engine tab), so its possible to do it that way in theory at least. Chickenpaint doesnt seem to like it.

I committed the changes so you can see. They require a fix on chickenpaint in order for saving as chi to cache to work

blurymind commented 3 years ago

Ok so I even tried placing the test.chi file in the public folder, its like chickenpaint doesnt even try to load it.

Are you sure this option actually does anything? It would really help if the example project actually used it. That way you would be able to test it for regressions

blurymind commented 3 years ago

Does it by any chance depend on resourcesRoot option? Is this file url absolute or relative to that?

blurymind commented 3 years ago

ahh grr, the option is loadChibiFileUrl, not loadChibiFileURL. its why it wasnt working. Sorry for the rant.

Still though, I seem to have found a bug with saving layered data. Would you like a PR for that?

thenickdude commented 3 years ago

const isImageLayer = layer instanceof CPImageLayer, it looks like it fails because it thinks its not an imageLayer, which triggers the thing that crashes it :(

how very odd, here is the console log, right before it image

This is because the two objects actually are different classes, because one was imported from the rollup pack and one was imported from the separated files. Import chickenpaint/js/ChickenPaint.js instead of just chickenpaint.

blurymind commented 3 years ago

Shouldn't packages.json be pointing to that then? Or at least have it noted what the proper way to import it is, since it is very specific or else some stuff wouldn't work

On Wed, 19 May 2021, 21:30 Nicholas Sherlock, @.***> wrote:

const isImageLayer = layer instanceof CPImageLayer, it looks like it fails because it thinks its not an imageLayer, which triggers the thing that crashes it :(

how very odd, here is the console log, right before it [image: image] https://user-images.githubusercontent.com/6495061/118862571-cc4bda00-b8d5-11eb-9841-ae4aad0fa813.png

This is because the two objects actually are different classes, because one was imported from the rollup pack and one was imported from the separated files. Import chickenpaint/js/ChickenPaint.js instead of just chickenpaint.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thenickdude/chickenpaint/issues/28#issuecomment-844449022, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVJVY2YT3FEBVXPUJN3TOQNWNANCNFSM45DB6HXQ .

thenickdude commented 3 years ago

Sure, I've pushed a new version 0.2.2 to set the "main" attribute of package.json now

thenickdude commented 3 years ago

Added a new section to the readme to explain what to include with NPM:

https://github.com/thenickdude/chickenpaint#usage---npm