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

Build improved screenshot API #1342

Open
pushfoo opened this issue Oct 2, 2022 · 11 comments · May be fixed by #2039
Open

Build improved screenshot API #1342

pushfoo opened this issue Oct 2, 2022 · 11 comments · May be fixed by #2039
Milestone

Comments

@pushfoo
Copy link
Member

pushfoo commented Oct 2, 2022

Enhancement request:

tl;dr add beginner-friendly screenshot functionality with reasonable defaults

What should be added/changed?

The minimum

A function that grabs the image from the screen and immediately writes it to disk. It should meet the following requirements:

  1. Calling with zero arguments saves a screenshot to the current working directory with a reasonable time stamp format such as "%windowname_%Y%m%d_%H%M_%S_%f.jpg"
  2. Accepts the same optional bounds arguments as get_image
  3. Accepts an optional save_path argument that directly specifies the directory to save to or filename to use, overriding any formatting in the latter case
  4. Accepts an optional time_format argument to override the default timestamp format.

Additional features

We may also want to add :

  • A screenshot manager object that allows setting a default filename / timestamp format
  • Queuing features that allow multiple screenshots to queued up before writing to disk at a specified time, such as on game exit to avoid slow disk access during gameplay
  • Integration with the pyglet event system (example: take a screenshot whenever event or condition X happens)
  • Integrations with video format libraries to automatically assemble a movie of the screenshots Moved to Video Capture in Arcade #1349

What would it help with?

  1. Reduce the number of tools users need to install or learn to get help
  2. Make the cross-platform experience much less confusing
  3. Make Documentation: Add guide on taking screenshots & recordings #1338 far easier to write
@pushfoo
Copy link
Member Author

pushfoo commented Oct 6, 2022

@einarf

It's possible to pipe each frame directly into ffmpeg instead. We don't have a wrapper for this, but it's not too difficult to make.

I noticed a previous commit removed ffmpeg as a windows dependency. Would we add it back in, or rely on a windows API for encoding?

@einarf
Copy link
Member

einarf commented Oct 6, 2022

I noticed a previous commit removed ffmpeg as a windows dependency. Would we add it back in, or rely on a windows API for encoding?

I think ffmpeg is something people would install separately. It's too much to bundle this for every platform.

@pushfoo
Copy link
Member Author

pushfoo commented Oct 6, 2022

It's too much to bundle this for every platform.

Which do you mean?

  • ffmpeg is overkill for this problem
  • managing ffmpeg as an included dependency creates too much work for the project
  • including ffmpeg makes the install size too large

@einarf
Copy link
Member

einarf commented Oct 6, 2022

Streaming raw frames into ffmpeg is very useful. A lot of other libraries do this (Wasabi2D, moderngl-window, etc)

The last option is true: including ffmpeg makes the install size too large

It's something you install on the side.

@pushfoo
Copy link
Member Author

pushfoo commented Oct 6, 2022

I'm worried that recommending ffmpeg installation might be out of the reach of beginner users. Does the following sound like a reasonable plan?

  1. Write an initial version of Documentation: Add guide on taking screenshots & recordings #1338 per the original plan of recommending OS built-ins whenever possible
  2. Implement the first half of this ticket in the near-term future, adding mention of it to the screenshot doc as useful for advanced users or debugging
  3. Build the screenshot queuing & video capture extensions whenever time arises, marking them in the doc as experimental or for advanced users

@einarf
Copy link
Member

einarf commented Oct 6, 2022

ffmpeg would probably not be a beginner thing. Can just start with screenshot and expand later. We don't even have the video capture module yet.

@pushfoo
Copy link
Member Author

pushfoo commented Oct 6, 2022

Having a unified & streamlined cross-platform video capture technique would significantly simplify the screen capture doc. We might want to revisit the topic at a later point after considering minimal & maximal install versions some more.

@pushfoo pushfoo changed the title Build improved screenshot / screen recording API Build improved screenshot API Oct 8, 2022
@pushfoo
Copy link
Member Author

pushfoo commented Nov 18, 2022

Which would be preferable for default save paths?

  1. Use the current working directory as the default save location
  2. Auto-detect a platform-specific save directory for screenshots such as ~/Pictures/
  3. Make save directory a mandatory argument, do not allow defaults

Option 1 would be fastest, so I'm in favor of implementing it first.

@einarf
Copy link
Member

einarf commented Nov 18, 2022

1 is more than fine.

@pushfoo
Copy link
Member Author

pushfoo commented Nov 20, 2022

I noticed another issue: get_image uses the top left as the origin while the rest of the library uses the bottom left.

I see two general ways of addressing it:

  • Leave get_image in the arcade namespace with the same signature and add new ^(get|save)_screenshot functions consistent with the coordinate system used elsewhere.
  • Use the existing names (get_image, get_pixel) and break compatibility.

Breaking compatibility seems like the better choice:

  1. The experimental examples call the method without arguments to capture the whole screen, as most users probably do.
  2. I haven't been able to come up with a legible alternate name for get_pixel.

This seems like a good approach for now:

  1. Make (get|save)_screenshot and get_pixel the official screenshot functions, with the versions in the arcade namespace calling those on the Window returned by get_window
  2. Leave the current get_(image|pixel) functions in draw_commands as deprecated legacy functions for anyone who still wants to import them directly.
  3. Update the programming guide and experimental examples accordingly.

It will make screenshots less confusing for all users and preserve the ability to use the non-OOP style used for teaching very young students.

@pushfoo
Copy link
Member Author

pushfoo commented Dec 10, 2022

We may want to wrap standard deprecation directives with warning boxes to mark get_image as deprecated. It seems that pyglet does something similar, but I haven't been able to find the exact place it defines that.

@einarf einarf added this to the Future milestone Mar 7, 2023
@sabadam32 sabadam32 linked a pull request Mar 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants