Add freeze functionality #72

AndreasBackx · 2023-10-25T02:19:00Z

Add freeze functionality

Stack created with Sapling. Best reviewed with ReviewStack.

Decodetalkers · 2023-10-27T10:49:51Z

Libwayshot also used in recording screen, it is not just used for screenshot. So when recording screen, it should not freeze itself,

Ok , I misunderstood this code. I think it will be fine

Decodetalkers · 2023-10-27T10:58:31Z

libwayshot/src/lib.rs

@@ -174,18 +167,19 @@ impl WayshotConnection {
        cursor_overlay: i32,
        output: &WlOutput,
        fd: T,
-        capture_region: Option<CaptureRegion>,
-    ) -> Result<FrameFormat> {
+    ) -> Result<(FrameFormat, FrameGuard)> {


Region is used in recording screen ,I do not think you should remove it

You're right, I'll keep support for requesting a screencopy for a specific region for the purposes of screen recording. I hadn't kept that in mind originally.

Decodetalkers · 2023-10-27T11:01:57Z

I think we should leave the exported functions have full features of wlr-screencopy, include user can pass region to capture an area. I think region , at least optional region is still needed to be a param

Decodetalkers · 2023-10-27T11:03:21Z

libwayshot/src/lib.rs

+        capture_region: CaptureRegion,
+        cursor_overlay: bool,
+    ) -> Result<RgbaImage> {
+        self.screenshot_region_capturer(RegionCapturer::Region(capture_region), cursor_overlay)


I don't think so? RegionCapturer is a new enum to differentiate what we're capturing:

pub enum RegionCapturer { Outputs(Vec<OutputInfo>), Region(CaptureRegion), Freeze(Box<dyn Fn() -> Result<CaptureRegion>>), }

Still need to do some documenting however to make this and the reasoning for it more clear!

Can you add a example above it ?

Will try to add an example of the freeze functionality. :)

AndreasBackx · 2023-10-28T16:14:43Z

I think we should leave the exported functions have full features of wlr-screencopy, include user can pass region to capture an area. I think region , at least optional region is still needed to be a param

I have so far not kept backwards compatibility in mind. Though my plan was to get it to work and think of a good API. Then see what we can do to keep it backwards compatible for now. I think these changes teach us how we can still make a breaking API change later, but for now it's best not to break every revision regardless of whether or not we're still in 0.x and we don't promise backwards compatibility. :)

Thanks for the review, will make necessary changes.

Decodetalkers · 2023-11-02T05:54:46Z

You have bring a file to commit

Decodetalkers · 2023-11-02T06:02:24Z

I think this pr is good.. but if there is some document is better, just no_run is ok

Decodetalkers · 2023-11-10T01:49:27Z

I do not think this pr is good enough... I have tried it.

First, I think freeze should be an option.
then, I have already use --slurp, but it still capture the whole screen and show it. I think if it is freeze, it should not need to be passed with slurp,

then the biggest problem is scale. when I use 2.0 scale or 1.5, the image cannot overfill my screen, it is terrible

Decodetalkers · 2023-11-10T01:52:00Z

wayshot/src/clap.rs

@@ -12,10 +12,10 @@ pub fn set_flags() -> Command {
                .help("Enable debug mode"),
        )
        .arg(


I think here should add on optional arg like freeze, because after freeze, we still need to do region select. so at first, we do not need to pass region to it

I can make a new PR to add something like --region <GEOMETRY> which would replicate the old --slurp. It would lead to RegionCapturer.Region(LogicalRegion) to be used instead of RegionCapturer.Freeze(...) and thus do any selection up front. I'd want to do this in a new PR because any changes I make here now will result in me having to do the work again because #76 moves us to Clap derive. Would you want me to add it there perhaps?

I think it's ok. Then solve the scale problem in this pr I think this pr is finished.

Decodetalkers · 2023-11-10T01:52:41Z

And you should try to set one of the screen to scale 1.2 or 2, then you will find the problem, scale is the most difficult thing to handle, I think , like flameshot cannot handle it propertly.. This feature is like flameshot, if scale can be propertly handle , it will be a greate feature

Decodetalkers · 2023-11-10T02:04:24Z

and make freeze to an option , another thing is , take screenshots between screens. if we cannot handle scale property, then it will make problem when take screenshot near the side of two screens

Decodetalkers

Sorry, I think this pr still have some problems. thanks for your efforts again

AndreasBackx · 2023-11-20T22:38:06Z

And you should try to set one of the screen to scale 1.2 or 2, then you will find the problem, scale is the most difficult thing to handle, I think , like flameshot cannot handle it propertly.. This feature is like flameshot, if scale can be propertly handle , it will be a greate feature

I have tried setting the scale to 1.2 which gives an error and something I am looking into. Could you explain the issue you see with a scale of 2 as I'm not immediately seeing an issue there? Could you provide the full command you use and the output you get vs what you expected?

Decodetalkers · 2023-11-21T01:31:29Z

Like this kind

Decodetalkers · 2023-11-21T01:32:18Z

And you should try to set one of the screen to scale 1.2 or 2, then you will find the problem, scale is the most difficult thing to handle, I think , like flameshot cannot handle it propertly.. This feature is like flameshot, if scale can be propertly handle , it will be a greate feature

I have tried setting the scale to 1.2 which gives an error and something I am looking into. Could you explain the issue you see with a scale of 2 as I'm not immediately seeing an issue there? Could you provide the full command you use and the output you get vs what you expected?

I have posted the image.. you should scale the captured to size of the screen

AndreasBackx · 2023-11-21T18:17:15Z

I have posted the image.. you should scale the captured to size of the screen

Thank you! That makes it very clear. Will have a look at it.

Decodetalkers · 2023-12-02T13:39:17Z

libwayshot/src/lib.rs

+                    output_info.wl_output.clone(),
+                );
+
+                layer_surface.set_exclusive_zone(-1);


You need the size from zxdg_output, not the size from frame_copy, the picture size by zwlr_copy is not always the size of the screen

Decodetalkers · 2023-12-02T13:41:54Z

libwayshot/src/dispatch.rs

+                width: _,
+                height: _,
+            } => {
+                tracing::debug!("Acking configure");


I think buffer attach and commit should be here

Decodetalkers · 2023-12-02T13:42:44Z

libwayshot/src/lib.rs

+
+                surface.set_buffer_transform(output_info.transform);
+                surface.set_buffer_scale(output_info.scale);
+                surface.attach(Some(&frame_guard.buffer), 0, 0);


before attach, you need to clone the buffer, and resize its size to the size of zwloutput, reasion is the same

…egion

This was referenced Oct 25, 2023

Refactor: introduce FrameGuard, ScreenCapturer, and Logical/EmbeddedRegion #71

Closed

Move from log to tracing #70

Closed

AndreasBackx force-pushed the pr72 branch 2 times, most recently from 9ddc27d to dc9c45b Compare October 25, 2023 21:42

Decodetalkers reviewed Oct 27, 2023

View reviewed changes

This was referenced Oct 29, 2023

refactor(image_composition): image::imageops::{overlay -> replace} #69

Closed

feat: add sleep option #63

Closed

AndreasBackx force-pushed the pr72 branch from dc9c45b to ddddb50 Compare October 31, 2023 00:57

AndreasBackx force-pushed the pr72 branch from ddddb50 to bbc7761 Compare November 7, 2023 22:38

AndreasBackx mentioned this pull request Nov 7, 2023

Move to clap_derive so CLI arguments are typed #75

Closed

Decodetalkers requested a review from Shinyzenith November 8, 2023 01:55

This was referenced Nov 8, 2023

Improve CLI design #76

Closed

Remove explicit panics and exits #77

Closed

Make region, sizing, and positioning data structs reusable #78

Merged

Decodetalkers reviewed Nov 10, 2023

View reviewed changes

Decodetalkers requested changes Nov 10, 2023

View reviewed changes

Decodetalkers reviewed Dec 2, 2023

View reviewed changes

AndreasBackx force-pushed the pr72 branch from bbc7761 to 255a685 Compare February 2, 2024 19:03

AndreasBackx mentioned this pull request Feb 2, 2024

tmp #84

Closed

Decodetalkers and others added 2 commits February 2, 2024 19:18

Refactor: introduce FrameGuard, ScreenCapturer, and Logical/EmbeddedR…

68a5c6a

…egion

Add freeze functionality

a023ff1

AndreasBackx force-pushed the pr72 branch from 255a685 to a023ff1 Compare February 2, 2024 19:21

Shinyzenith closed this Feb 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add freeze functionality #72

Add freeze functionality #72

AndreasBackx commented Oct 25, 2023 •

edited

Loading

Decodetalkers commented Oct 27, 2023 •

edited

Loading

Decodetalkers Oct 27, 2023

AndreasBackx Oct 28, 2023

Decodetalkers commented Oct 27, 2023

Decodetalkers Oct 27, 2023

AndreasBackx Oct 28, 2023

Decodetalkers Nov 2, 2023

AndreasBackx Nov 3, 2023

AndreasBackx commented Oct 28, 2023

Decodetalkers commented Nov 2, 2023 •

edited

Loading

Decodetalkers commented Nov 2, 2023

Decodetalkers commented Nov 10, 2023

Decodetalkers Nov 10, 2023

AndreasBackx Nov 20, 2023

Decodetalkers Nov 21, 2023

Decodetalkers commented Nov 10, 2023 •

edited

Loading

Decodetalkers commented Nov 10, 2023

Decodetalkers left a comment •

edited

Loading

AndreasBackx commented Nov 20, 2023

Decodetalkers commented Nov 21, 2023

Decodetalkers commented Nov 21, 2023

AndreasBackx commented Nov 21, 2023

Decodetalkers Dec 2, 2023

Decodetalkers Dec 2, 2023

Decodetalkers Dec 2, 2023 •

edited

Loading

Add freeze functionality #72

Add freeze functionality #72

Conversation

AndreasBackx commented Oct 25, 2023 • edited Loading

Decodetalkers commented Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Decodetalkers commented Oct 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreasBackx commented Oct 28, 2023

Decodetalkers commented Nov 2, 2023 • edited Loading

Decodetalkers commented Nov 2, 2023

Decodetalkers commented Nov 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Decodetalkers commented Nov 10, 2023 • edited Loading

Decodetalkers commented Nov 10, 2023

Decodetalkers left a comment • edited Loading

Choose a reason for hiding this comment

AndreasBackx commented Nov 20, 2023

Decodetalkers commented Nov 21, 2023

Decodetalkers commented Nov 21, 2023

AndreasBackx commented Nov 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Decodetalkers Dec 2, 2023 • edited Loading

Choose a reason for hiding this comment

AndreasBackx commented Oct 25, 2023 •

edited

Loading

Decodetalkers commented Oct 27, 2023 •

edited

Loading

Decodetalkers commented Nov 2, 2023 •

edited

Loading

Decodetalkers commented Nov 10, 2023 •

edited

Loading

Decodetalkers left a comment •

edited

Loading

Decodetalkers Dec 2, 2023 •

edited

Loading