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

Load saved file #311

Merged
merged 23 commits into from
Apr 13, 2020
Merged

Load saved file #311

merged 23 commits into from
Apr 13, 2020

Conversation

Alecaddd
Copy link
Member

@Alecaddd Alecaddd commented Mar 25, 2020

Summary / How this PR fixes the problem?

Implement the load of a previously saved file.

This PR fixes/implements the following bugs/features:

  • Open a previously saved file on a dedicated Window.
  • Load all the Artboards.
  • Load all the Items.
  • Restore the previous selection status.

Fixes

@Alecaddd Alecaddd added this to the v1.0 milestone Mar 25, 2020
@Alecaddd Alecaddd requested review from giacomoalbe and albfan and removed request for giacomoalbe March 25, 2020 06:34
@Alecaddd
Copy link
Member Author

@giacomoalbe @albfan hey guys, I pretty much have this patch ready, except for an issue that I can't figure out why it's happening.

If a shape is inside an artboard, after it gets loaded, is not possible to move it. The selection bound moves, the resize works, and moving with the arrow keys work, but not with the mouse.
Can you give it a look?

To test it, create an arboard and a shape inside, save the file, close Akira and load that file.
Everything should be in place, but if you try to move that shape with the mouse, no redraw happens.

@Alecaddd Alecaddd changed the title [WIP] Load saved file Load saved file Mar 25, 2020
@Alecaddd Alecaddd marked this pull request as ready for review March 25, 2020 06:37
@Alecaddd
Copy link
Member Author

Another issue solved, but another issue found.
It seems that my int_to_rgba method is not correct and the colors are not properly loaded.
Can you guys give it a look?

@Alecaddd
Copy link
Member Author

I fixed it by saving the RGBA colors in a string format and creating a dedicated method to load them again.

@Alecaddd
Copy link
Member Author

Here's a file example you can use to test this patch.
3-artboards.akira.zip

And this is how the app should look once loaded, including remembering the currently selected item.
Screenshot from 2020-03-26 21-36-12

@giacomoalbe
Copy link
Contributor

I checked out this PR locally and did some tests, but I haven't been able to make the Artboard children to work. I don't know whether it is a known bug or not, but everything seems to be working except for the Artboard children stuff.

I have those critical errors for the Artboard's children layers:

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_add_child_item: assertion 'self != NULL' failed

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_get_child_item: assertion 'self != NULL' failed

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_add_child_item: assertion 'self != NULL' failed

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_get_child_item: assertion 'self != NULL' failed

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_add_child_item: assertion 'self != NULL' failed

(Akira:11529): Akira-CRITICAL **: 23:01:09.090: akira_models_layer_model_get_child_item: assertion 'self != NULL' failed

And here's a screenshot of the Layers panel after the file import:

image

As you can see, all the Artboard are expanded, so the children should be visible, but they're not.

Idk if this is due to the fact that the Layers panel and ItemsManager are not updated to the Master branch, but as of now they're not working as expected.

@Alecaddd
Copy link
Member Author

Alecaddd commented Apr 6, 2020

@giacomoalbe yes, this is normal as the issue is due to the incompatibility with the master branch.
After your latest PR landed, the list model for the free items is different and doesn't actually save anything.
I'll rebase this later this week.

@Alecaddd
Copy link
Member Author

@albfan @giacomoalbe this is ready for a full review.

I stumbled upon some quirks which I'm not totally sure why.
Specifically, I had to manually set the item.bounds of a loaded Artboard, otherwise those would be 0 when a new item is created and we need to check if the item is inside an Artboard.

case "AkiraLibModelsCanvasArtboard":
insert_type = Models.CanvasItemType.ARTBOARD;
item = insert_item (pos_x, pos_y);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I can see the CanvasImage item's loading is not yet implemented.

I don't know if this is due to the fact the we have a separated PR for images and that would have caused merge issue, so I state this here as a reminder!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I left the image stuff outside as I'm not yet saving images at all.
I will implement this in a separate PR.

@giacomoalbe
Copy link
Contributor

Ok, for me this code is super ok! I just need to take a more detailed look at it (and maybe start tinkering with it) in order to properly understand each stage involved in the process, but overall I think it's very well written and concise!

@Alecaddd Alecaddd merged commit 737a3f0 into master Apr 13, 2020
@Alecaddd Alecaddd deleted the load-saved-file branch April 13, 2020 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants