Page MenuHome

File Close Dialog
ClosedPublic

Authored by Jacques Lucke (JacquesLucke) on May 14 2019, 6:15 PM.

Details

Summary

This adds a new dialog that is shown when a file is closed (either when a new file is loaded or when Blender quits).

The "Save Images" checkbox does nothing at the moment.

Diff Detail

Repository
rB Blender
Branch
file-close-dialog (branched from master)
Build Status
Buildable 3585
Build 3585: arc lint + arc unit

Event Timeline

  • save images checkbox
  • fix crash when closing popup
  • fix memory leak

It's extremely confusing that IDP_FreeProperty does not free
everything IDP_CopyProperty allocates...

  • remove now unnecessary free
  • Simplify calling an operator with given properties
  • better function name
Jacques Lucke (JacquesLucke) retitled this revision from File Close Dialog [WIP] to File Close Dialog.May 16 2019, 2:57 PM
Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)
Jacques Lucke (JacquesLucke) updated this revision to Diff 15396.
  • actually save modified images

I noticed two minor issues:

  • The .blend is not tagged dirty when I just draw on an image and don't change anything else.
  • I think it would be better if it would check for duplicate file paths first, before saving any images. Currently, it would work on the second quit attempt, because the first one is saved already.
  • reduce vertical scope

I noticed two minor issues:

  • The .blend is not tagged dirty when I just draw on an image and don't change anything else.

Right, we'll need to call WM_file_tag_modified when marking images dirty.

  • I think it would be better if it would check for duplicate file paths first, before saving any images. Currently, it would work on the second quit attempt, because the first one is saved already.

That's a good point, we can put those errors into the dialog so users can decide to fix them first, or ignore them.

I noticed two minor issues:

  • The .blend is not tagged dirty when I just draw on an image and don't change anything else.

Right, we'll need to call WM_file_tag_modified when marking images dirty.

  • I think it would be better if it would check for duplicate file paths first, before saving any images. Currently, it would work on the second quit attempt, because the first one is saved already.

That's a good point, we can put those errors into the dialog so users can decide to fix them first, or ignore them.

Will you do that? If not, then I'll implement it tomorrow.

Brecht Van Lommel (brecht) requested changes to this revision.May 16 2019, 6:42 PM

Seems to work great, I have only minor nitpicks.

source/blender/windowmanager/intern/wm_files.c
2944

Leave out the question mark, can also do this:

BLI_snprintf(message, sizeof(message), (modified_images_count == 1) ? "Save %u modified image" : "Save %u modified images", modified_images_count);
2961

Add uiItemL(layout, "", ICON_NONE); for empty line here.

The text is too close to the buttons now.

This revision now requires changes to proceed.May 16 2019, 6:42 PM

Will you do that? If not, then I'll implement it tomorrow.

I'll let you do it.

I did a little bit of the work in rB4c4ac1158bca: Images: more tweaks to save all modified images, so now you can get the warnings in advance with the API function. I'll leave the UI to you.

  • fix after rebase
  • extra space between text and buttons
  • separate dry run from actual saving
  • display reports in dialog

We are still missing tagging the .blend as modified when an image changes, but otherwise looks good.

This revision is now accepted and ready to land.May 17 2019, 4:32 PM

This is a fantastic step forward. File->New and File->Open now properly show the same consistent dialog as quitting:

Small note that, at least on Mac, this dialog does not appear when actually closing the window using the native close button. In that case you still get this popup:

...Which is not as useful, because you cannot actually save or do anything from there, other than cancel the operation.

We should indeed this dialog on all platforms now.

With testing, I also found that, if you add a new image inside of Blender, it does not get saved if it doesn't have a file path. As I understand it, the plan is to eventually make it automatically pack into the blend?

That can be done as a second step after this is committed I suppose.

  • show dialog as well when only images have changed

With testing, I also found that, if you add a new image inside of Blender, it does not get saved if it doesn't have a file path. As I understand it, the plan is to eventually make it automatically pack into the blend?
That can be done as a second step after this is committed I suppose.

That's the part I'm working on, it will be a separate change.

William Reynish (billreynish) requested changes to this revision.May 17 2019, 5:32 PM

I noticed one small issue: the Save icon is wrong here. It uses the same icon as Discard Changes.

Personally I would not even use any icons here - none of the native OS dialogs seem to do that anyway.

This revision now requires changes to proceed.May 17 2019, 5:32 PM
This revision was not accepted when it landed; it landed in state Needs Revision.May 17 2019, 5:44 PM
This revision was automatically updated to reflect the committed changes.

It would be nice if this dialog could support keyboard navigation (Left and Right arrow keys)...

@TheRedWaxPolice (TheRedWaxPolice)

You can use it via the keyboard:

Save: Return
Cancel: Escape

Additionally, you can use C, D, and S for Cancel, Discard and Save.

@William Reynish (billreynish) Those direct shortcuts are a good alternative, but I was specifically referring to the left and right arrow keys, because it's a common feature on those type of dialogs.

It's not a big deal, it's just unusual not supporting that.