Eliminate blocking in Error() and Message() calls.

This serves two purposes.

First, we want to (some day) convert these messages into a less
obtrustive form, something like toaster notifications, such that they
don't interrupt workflow as harshly. That would, of course, be
nonblocking.

Second, some platforms, like Emscripten, do not support nested event
loops, and it's not possible to display a modal dialog on them
synchronously.

When making this commit, I've reviewed all Error() and Message()
calls to ensure that only some of the following is true for all
of them:
  * The call is followed a break or return statement that exits
    an UI entry point (e.g. an MenuX function);
  * The call is followed by cleanup (in fact, in this case the new
    behavior is better, since even with a synchronous modal dialog
    we have to be reentrant);
  * The message is an informational message only and nothing
    unexpected will happen if the operation proceeds in background.

In general, all Error() calls already satisfied the above conditions,
although in some cases I changed control flow aroudn them to more
clearly show that. The Message() calls that didn't satisfy these
conditions were reworked into an asynchronous form.

There are three explicit RunModal() calls left that need to be
reworked into an async form.
pull/339/head
whitequark 2018-07-19 21:54:05 +00:00
parent 70177166d7
commit 8426992f27
10 changed files with 88 additions and 33 deletions

View File

@ -331,10 +331,10 @@ bool TextWindow::EditControlDoneForConfiguration(const std::string &s) {
double x, y, z; double x, y, z;
if(sscanf(s.c_str(), "%lf, %lf, %lf", &x, &y, &z)==3) { if(sscanf(s.c_str(), "%lf, %lf, %lf", &x, &y, &z)==3) {
SS.lightDir[edit.i] = Vector::From(x, y, z); SS.lightDir[edit.i] = Vector::From(x, y, z);
SS.GW.Invalidate();
} else { } else {
Error(_("Bad format: specify coordinates as x, y, z")); Error(_("Bad format: specify coordinates as x, y, z"));
} }
SS.GW.Invalidate();
break; break;
} }
case Edit::COLOR: { case Edit::COLOR: {
@ -367,11 +367,11 @@ bool TextWindow::EditControlDoneForConfiguration(const std::string &s) {
} }
case Edit::CAMERA_TANGENT: { case Edit::CAMERA_TANGENT: {
SS.cameraTangent = (min(2.0, max(0.0, atof(s.c_str()))))/1000.0; SS.cameraTangent = (min(2.0, max(0.0, atof(s.c_str()))))/1000.0;
SS.GW.Invalidate();
if(!SS.usePerspectiveProj) { if(!SS.usePerspectiveProj) {
Message(_("The perspective factor will have no effect until you " Message(_("The perspective factor will have no effect until you "
"enable View -> Use Perspective Projection.")); "enable View -> Use Perspective Projection."));
} }
SS.GW.Invalidate();
break; break;
} }
case Edit::GRID_SPACING: { case Edit::GRID_SPACING: {
@ -385,8 +385,8 @@ bool TextWindow::EditControlDoneForConfiguration(const std::string &s) {
Error(_("Specify between 0 and 8 digits after the decimal.")); Error(_("Specify between 0 and 8 digits after the decimal."));
} else { } else {
SS.SetUnitDigitsAfterDecimal(v); SS.SetUnitDigitsAfterDecimal(v);
SS.GW.Invalidate();
} }
SS.GW.Invalidate();
break; break;
} }
case Edit::EXPORT_SCALE: { case Edit::EXPORT_SCALE: {

View File

@ -844,6 +844,7 @@ static Platform::MessageDialog::Response LocateImportedFile(const Platform::Path
dialog->AddButton(C_("button", "&Cancel"), MessageDialog::Response::CANCEL); dialog->AddButton(C_("button", "&Cancel"), MessageDialog::Response::CANCEL);
} }
// FIXME(async): asyncify this call
return dialog->RunModal(); return dialog->RunModal();
} }

View File

@ -678,15 +678,17 @@ void GraphicsWindow::MenuView(Command id) {
case Command::SHOW_GRID: case Command::SHOW_GRID:
SS.GW.showSnapGrid = !SS.GW.showSnapGrid; SS.GW.showSnapGrid = !SS.GW.showSnapGrid;
SS.GW.EnsureValidActives();
SS.GW.Invalidate();
if(SS.GW.showSnapGrid && !SS.GW.LockedInWorkplane()) { if(SS.GW.showSnapGrid && !SS.GW.LockedInWorkplane()) {
Message(_("No workplane is active, so the grid will not appear.")); Message(_("No workplane is active, so the grid will not appear."));
} }
SS.GW.EnsureValidActives();
SS.GW.Invalidate();
break; break;
case Command::PERSPECTIVE_PROJ: case Command::PERSPECTIVE_PROJ:
SS.usePerspectiveProj = !SS.usePerspectiveProj; SS.usePerspectiveProj = !SS.usePerspectiveProj;
SS.GW.EnsureValidActives();
SS.GW.Invalidate();
if(SS.cameraTangent < 1e-6) { if(SS.cameraTangent < 1e-6) {
Error(_("The perspective factor is set to zero, so the view will " Error(_("The perspective factor is set to zero, so the view will "
"always be a parallel projection.\n\n" "always be a parallel projection.\n\n"
@ -694,8 +696,6 @@ void GraphicsWindow::MenuView(Command id) {
"factor in the configuration screen. A value around 0.3 " "factor in the configuration screen. A value around 0.3 "
"is typical.")); "is typical."));
} }
SS.GW.EnsureValidActives();
SS.GW.Invalidate();
break; break;
case Command::ONTO_WORKPLANE: case Command::ONTO_WORKPLANE:
@ -1050,11 +1050,11 @@ void GraphicsWindow::MenuEdit(Command id) {
} }
} }
} while(didSomething); } while(didSomething);
SS.GW.Invalidate();
SS.ScheduleShowTW();
if(newlySelected == 0) { if(newlySelected == 0) {
Error(_("No additional entities share endpoints with the selected entities.")); Error(_("No additional entities share endpoints with the selected entities."));
} }
SS.GW.Invalidate();
SS.ScheduleShowTW();
break; break;
} }
@ -1077,7 +1077,6 @@ void GraphicsWindow::MenuEdit(Command id) {
break; break;
} }
SS.UndoRemember(); SS.UndoRemember();
// Rotate by ninety degrees about the coordinate axis closest // Rotate by ninety degrees about the coordinate axis closest
// to the screen normal. // to the screen normal.
@ -1168,20 +1167,18 @@ void GraphicsWindow::MenuRequest(Command id) {
} else if(g->type == Group::Type::DRAWING_WORKPLANE) { } else if(g->type == Group::Type::DRAWING_WORKPLANE) {
// The group's default workplane // The group's default workplane
g->activeWorkplane = g->h.entity(0); g->activeWorkplane = g->h.entity(0);
Message(_("No workplane selected. Activating default workplane " MessageAndRun([] {
"for this group.")); // Align the view with the selected workplane
} SS.GW.ClearSuper();
SS.GW.AnimateOntoWorkplane();
if(!SS.GW.LockedInWorkplane()) { }, _("No workplane selected. Activating default workplane "
"for this group."));
} else {
Error(_("No workplane is selected, and the active group does " Error(_("No workplane is selected, and the active group does "
"not have a default workplane. Try selecting a " "not have a default workplane. Try selecting a "
"workplane, or activating a sketch-in-new-workplane " "workplane, or activating a sketch-in-new-workplane "
"group.")); "group."));
break;
} }
// Align the view with the selected workplane
SS.GW.ClearSuper();
SS.GW.AnimateOntoWorkplane();
break; break;
} }
case Command::FREE_IN_3D: case Command::FREE_IN_3D:
@ -1232,12 +1229,13 @@ c:
break; break;
case Command::CONSTRUCTION: { case Command::CONSTRUCTION: {
SS.UndoRemember();
SS.GW.GroupSelection(); SS.GW.GroupSelection();
if(SS.GW.gs.entities == 0) { if(SS.GW.gs.entities == 0) {
Error(_("No entities are selected. Select entities before " Error(_("No entities are selected. Select entities before "
"trying to toggle their construction state.")); "trying to toggle their construction state."));
break;
} }
SS.UndoRemember();
int i; int i;
for(i = 0; i < SS.GW.gs.entities; i++) { for(i = 0; i < SS.GW.gs.entities; i++) {
hEntity he = SS.GW.gs.entity[i]; hEntity he = SS.GW.gs.entity[i];

View File

@ -1100,6 +1100,7 @@ static void ImportDwgDxf(const Platform::Path &filename,
importer.clearBlockTransform(); importer.clearBlockTransform();
if(!read(data, &importer)) { if(!read(data, &importer)) {
Error("Corrupted %s file.", fileType.c_str()); Error("Corrupted %s file.", fileType.c_str());
return;
} }
if(importer.unknownEntities > 0) { if(importer.unknownEntities > 0) {
Message("%u %s entities of unknown type were ignored.", Message("%u %s entities of unknown type were ignored.",

View File

@ -711,6 +711,7 @@ void GraphicsWindow::SplitLinesOrCurves() {
} }
} else { } else {
Error(_("Can't split; no intersection found.")); Error(_("Can't split; no intersection found."));
return;
} }
// All done, clean up and regenerate. // All done, clean up and regenerate.

View File

@ -213,7 +213,7 @@ public:
std::function<void()> onClose; std::function<void()> onClose;
std::function<void(bool)> onFullScreen; std::function<void(bool)> onFullScreen;
std::function<bool(MouseEvent)> onMouseEvent; std::function<bool(MouseEvent)> onMouseEvent;
std::function<void(SixDofEvent)> onSixDofEvent; std::function<void(SixDofEvent)> onSixDofEvent;
std::function<bool(KeyboardEvent)> onKeyboardEvent; std::function<bool(KeyboardEvent)> onKeyboardEvent;
std::function<void(std::string)> onEditingDone; std::function<void(std::string)> onEditingDone;
std::function<void(double)> onScrollbarAdjusted; std::function<void(double)> onScrollbarAdjusted;
@ -293,6 +293,8 @@ public:
CANCEL CANCEL
}; };
std::function<void(Response)> onResponse;
virtual ~MessageDialog() {} virtual ~MessageDialog() {}
virtual void SetType(Type type) = 0; virtual void SetType(Type type) = 0;
@ -303,6 +305,12 @@ public:
virtual void AddButton(std::string name, Response response, bool isDefault = false) = 0; virtual void AddButton(std::string name, Response response, bool isDefault = false) = 0;
virtual Response RunModal() = 0; virtual Response RunModal() = 0;
virtual void ShowModal() {
Response response = RunModal();
if(onResponse) {
onResponse(response);
}
}
}; };
typedef std::shared_ptr<MessageDialog> MessageDialogRef; typedef std::shared_ptr<MessageDialog> MessageDialogRef;

View File

@ -1069,7 +1069,12 @@ void Request3DConnexionEventsForWindow(WindowRef window) {}
// Message dialogs // Message dialogs
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
class MessageDialogImplGtk final : public MessageDialog { class MessageDialogImplGtk;
static std::vector<std::shared_ptr<MessageDialogImplGtk>> shownMessageDialogs;
class MessageDialogImplGtk final : public MessageDialog,
public std::enable_shared_from_this<MessageDialogImplGtk> {
public: public:
Gtk::Image gtkImage; Gtk::Image gtkImage;
Gtk::MessageDialog gtkDialog; Gtk::MessageDialog gtkDialog;
@ -1078,7 +1083,16 @@ public:
: gtkDialog(parent, "", /*use_markup=*/false, Gtk::MESSAGE_INFO, : gtkDialog(parent, "", /*use_markup=*/false, Gtk::MESSAGE_INFO,
Gtk::BUTTONS_NONE, /*modal=*/true) Gtk::BUTTONS_NONE, /*modal=*/true)
{ {
SetTitle("Message"); SetTitle("Message");
gtkDialog.signal_response().connect([this](int gtkResponse) {
ProcessResponse(gtkResponse);
});
gtkDialog.signal_hide().connect([this] {
auto it = std::remove(shownMessageDialogs.begin(), shownMessageDialogs.end(),
shared_from_this());
shownMessageDialogs.erase(it);
});
} }
void SetType(Type type) override { void SetType(Type type) override {
@ -1129,21 +1143,36 @@ public:
} }
} }
Response RunModal() override { Response ProcessResponse(int gtkResponse) {
switch(gtkDialog.run()) { Response response;
case Gtk::RESPONSE_OK: return Response::OK; break; switch(gtkResponse) {
case Gtk::RESPONSE_YES: return Response::YES; break; case Gtk::RESPONSE_OK: response = Response::OK; break;
case Gtk::RESPONSE_NO: return Response::NO; break; case Gtk::RESPONSE_YES: response = Response::YES; break;
case Gtk::RESPONSE_CANCEL: return Response::CANCEL; break; case Gtk::RESPONSE_NO: response = Response::NO; break;
case Gtk::RESPONSE_CANCEL: response = Response::CANCEL; break;
case Gtk::RESPONSE_NONE: case Gtk::RESPONSE_NONE:
case Gtk::RESPONSE_CLOSE: case Gtk::RESPONSE_CLOSE:
case Gtk::RESPONSE_DELETE_EVENT: case Gtk::RESPONSE_DELETE_EVENT:
return Response::NONE; response = Response::NONE;
break; break;
default: ssassert(false, "Unexpected response"); default: ssassert(false, "Unexpected response");
} }
if(onResponse) {
onResponse(response);
}
return response;
}
void ShowModal() override {
shownMessageDialogs.push_back(shared_from_this());
gtkDialog.show();
}
Response RunModal() override {
return ProcessResponse(gtkDialog.run());
} }
}; };

View File

@ -161,6 +161,7 @@ bool SolveSpaceUI::LoadAutosaveFor(const Platform::Path &filename) {
/*isDefault=*/true); /*isDefault=*/true);
dialog->AddButton(C_("button", "Do&n't Load"), MessageDialog::Response::NO); dialog->AddButton(C_("button", "Do&n't Load"), MessageDialog::Response::NO);
// FIXME(async): asyncify this call
if(dialog->RunModal() == MessageDialog::Response::YES) { if(dialog->RunModal() == MessageDialog::Response::YES) {
unsaved = true; unsaved = true;
return LoadFromFile(autosaveFile, /*canCancel=*/true); return LoadFromFile(autosaveFile, /*canCancel=*/true);
@ -463,6 +464,7 @@ bool SolveSpaceUI::OkayToStartNewFile() {
dialog->AddButton(C_("button", "Do&n't Save"), MessageDialog::Response::NO); dialog->AddButton(C_("button", "Do&n't Save"), MessageDialog::Response::NO);
dialog->AddButton(C_("button", "&Cancel"), MessageDialog::Response::CANCEL); dialog->AddButton(C_("button", "&Cancel"), MessageDialog::Response::CANCEL);
// FIXME(async): asyncify this call
switch(dialog->RunModal()) { switch(dialog->RunModal()) {
case MessageDialog::Response::YES: case MessageDialog::Response::YES:
return GetFilenameAndSave(/*saveAs=*/false); return GetFilenameAndSave(/*saveAs=*/false);

View File

@ -235,6 +235,7 @@ void MultMatrix(double *mata, double *matb, double *matr);
int64_t GetMilliseconds(); int64_t GetMilliseconds();
void Message(const char *fmt, ...); void Message(const char *fmt, ...);
void MessageAndRun(std::function<void()> onDismiss, const char *fmt, ...);
void Error(const char *fmt, ...); void Error(const char *fmt, ...);
class System { class System {

View File

@ -100,10 +100,11 @@ void SolveSpace::MultMatrix(double *mata, double *matb, double *matr) {
} }
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
// Word-wrap the string for our message box appropriately, and then display // Format the string for our message box appropriately, and then display
// that string. // that string.
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
static void MessageBox(const char *fmt, va_list va, bool error) static void MessageBox(const char *fmt, va_list va, bool error,
std::function<void()> onDismiss = std::function<void()>())
{ {
#ifndef LIBRARY #ifndef LIBRARY
va_list va_size; va_list va_size;
@ -156,7 +157,13 @@ static void MessageBox(const char *fmt, va_list va, bool error)
} }
dialog->AddButton(C_("button", "&OK"), MessageDialog::Response::OK, dialog->AddButton(C_("button", "&OK"), MessageDialog::Response::OK,
/*isDefault=*/true); /*isDefault=*/true);
dialog->RunModal();
dialog->onResponse = [=](MessageDialog::Response _response) {
if(onDismiss) {
onDismiss();
}
};
dialog->ShowModal();
#endif #endif
} }
void SolveSpace::Error(const char *fmt, ...) void SolveSpace::Error(const char *fmt, ...)
@ -173,6 +180,13 @@ void SolveSpace::Message(const char *fmt, ...)
MessageBox(fmt, f, /*error=*/false); MessageBox(fmt, f, /*error=*/false);
va_end(f); va_end(f);
} }
void SolveSpace::MessageAndRun(std::function<void()> onDismiss, const char *fmt, ...)
{
va_list f;
va_start(f, fmt);
MessageBox(fmt, f, /*error=*/false, onDismiss);
va_end(f);
}
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
// Solve a mostly banded matrix. In a given row, there are LEFT_OF_DIAG // Solve a mostly banded matrix. In a given row, there are LEFT_OF_DIAG