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

pixDisplayWithTitle(): supports 'open' mode on Win32/64 + supports 'store' mode for all #683

Closed

Conversation

GerHobbelt
Copy link
Contributor

pixDisplayWithTitle():

  • supports 'open' mode on Win32/64 (opens the system-registered image viewer application via explorer /open,%1
  • additional modes for all platforms:
    • NONE (disables display entirely)
    • STORE (stores the image to /tmp/lept/disp/ as is done in the other modes, but DOES NOT invoke any external application for viewing, i.e. no system() call.

- supports 'open' mode on Win32/64 (opens the system-registered image viewer application via `explorer /open,%1`
- additional modes for all platforms:
  + NONE  (disables display entirely)
  + STORE (stores the image to /tmp/lept/disp/ as is done in the other modes, but DOES NOT invoke any external application for viewing, i.e. no `system()` call.
@GerHobbelt
Copy link
Contributor Author

😢 NONE mode is currently equiv. to STORE mode. fix commit will be added to his pullreq. 😊

…hs the description: no store, no display, nothing.
@DanBloomberg
Copy link
Owner

I don't see the point in most of this.

We have a global L_DISPLAY_WITH_... that can be set with l_chooseDisplayProg(),
but for most situations the default suffices. If you don't want output, use
0 for the dispflag in pixDisplayWithTitle -- we don't need to call
l_chooseDisplayProg(L_DISPLAY_WITH_NONE) first. If you want to write the
image to file, use pixWrite() -- there's no good way to do it from
pixDisplay(), and your code doesn't do it either. There is no place to
specify the output filename.

Did you mean to have the default windows display program be "open"?
If so, why that and why not

For windows, what is wrong with getting the path on the stack with:

   pathname = genPathname(tempname, NULL);
   _fullpath(fullpath, pathname, sizeof(fullpath));

?

For Windows, we can allow use of the 'open' display program with this
variant of your code:

        /* Windows: L_DISPLAY_WITH_IV or L_DISPLAY_WITH_OPEN */
    pathname = genPathname(tempname, NULL);
    _fullpath(fullpath, pathname, sizeof(fullpath));
    if (var_DISPLAY_PROG == L_DISPLAY_WITH_IV) {
        if (title) {
            snprintf(buffer, Bufsize,
                     "i_view32.exe \"%s\" /pos=(%d,%d) /title=\"%s\"",
                     fullpath, x, y, title);
        } else {
            snprintf(buffer, Bufsize, "i_view32.exe \"%s\" /pos=(%d,%d)",
                     fullpath, x, y);
        }
    } else {  /* L_DISPLAY_WITH_OPEN */
        snprintf(buffer, Bufsize, "explorer.exe /open \"%s\"", fulllpath);
    }

Interesting syntax with the comma in the command line:
explorer.exe /open,"%s"

I'll add checks at the top of the function to make sure that the display program is valid for the platform.

@GerHobbelt
Copy link
Contributor Author

GerHobbelt commented Mar 20, 2023 via email

@GerHobbelt
Copy link
Contributor Author

GerHobbelt commented Mar 20, 2023 via email

@DanBloomberg
Copy link
Owner

Thank you for the information about how Windows calls display programs.

And you're right, it would be useful to be able to disable pixDisplay() calls!

@DanBloomberg
Copy link
Owner

commit 0ecd513
Please let me know if is is now working properly for you.

Dan

@GerHobbelt
Copy link
Contributor Author

👍 merged and working fine.

P.S. I like my STORE mode (so I can run /prog/*, etc. in batch mode without viewers popping up into my face, but still collect the images they were supposed to display), so I merged & mixed it back into my own codebase. But that's just me; won't be all that useful to others.

@GerHobbelt
Copy link
Contributor Author

Shall I close this as completed, then?

@GerHobbelt GerHobbelt closed this Mar 21, 2023
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