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

fix(layer-shell-scaling): frozen buffers must scale to output scaling. #112

Closed

Conversation

Decodetalkers
Copy link
Collaborator

I have read the portocol of fractional, the protocal just accept a scale value to client and let client to adjust the scale. so finally the way to scale is done by client. So the scale should be done by ourself

@Decodetalkers
Copy link
Collaborator Author

and now in the code , the way to use slurp is a wrong way

@Decodetalkers
Copy link
Collaborator Author

oh.. clippy is merged.. emm, so it is the problem that I have not find my pr

@Shinyzenith
Copy link
Member

Shinyzenith commented Mar 28, 2024

Do we have to do another buffer allocation for the surface layering? Is it not possible to bilinearly scale up/down the current surface contents?

Scratch that, I think we already have pixel perfect version from the comp in the buffer. We just need to set the scale hint.

@Decodetalkers
Copy link
Collaborator Author

Decodetalkers commented Mar 28, 2024

Do we have to do another buffer allocation for the surface layering? Is it not possible to bilinearly scale up/down the current surface contents?

the size which is used to scale the buffer only accept int, so when the scale is int, I will use the origin one.. but when it is fractional one.. I cannot find out a solution. the protocal about fractional just throw a size to let client to resize it, do you have better idea?

@Shinyzenith Shinyzenith changed the title finall I think the scale problem should fixed by ourself fix(layer-shell-scaling): frozen buffers must scale to output scaling. Mar 28, 2024
@Shinyzenith
Copy link
Member

Do we have to do another buffer allocation for the surface layering? Is it not possible to bilinearly scale up/down the current surface contents?

the size which is used to scale the buffer only accept int, so when the scale is int, I will use the origin one.. but when it is fractional one.. I cannot find out a solution. the protocal about fractional just throw a size to let client to resize it, do you have better idea?

I sadly don't. As far as I'm aware, screencopy already returns to us a pixel-perfect version which is scaled appropriately due to Andreas's patches, we just need a way to hint to the compositor that we have the scaled version already. I'm not sure if my understanding is correct. I'll take a look at the code in the evening and maybe ask in sway-irc if needed.

Copy link
Member

@AndreasBackx AndreasBackx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like @Shinyzenith says (and see my other review comment), the approach is incorrect. We already have a pixel perfect buffer that needs no changes.

Tried this on my machine and it only results in an even blurrier version of what happened before which was already incorrect.

let buffer = {
let output_size = output_info.logical_region.inner.size;
if output_size != frame_copy.frame_format.size {
let file = tempfile::tempfile().unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let file = tempfile::tempfile().unwrap();
let file = tempfile::tempfile()?;

If not the correct error type, wrap it in another type, don't unwrap.

Comment on lines 463 to 492
let image = image::imageops::resize(
&image,
output_size.width,
output_size.height,
image::imageops::FilterType::Triangle,
);
init_overlay(&image, &file);
let pool = shm.create_pool(
file.as_fd(),
frame_copy
.frame_format
.byte_size()
.try_into()
.map_err(|_| Error::BufferTooSmall)?,
&qh,
(),
);

pool.create_buffer(
0,
output_size.width as i32,
output_size.height as i32,
(output_size.width * 4) as i32,
frame_copy.frame_format.format,
&qh,
(),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no reason to resize. We have a pixel perfect buffer already that represents every single pixel shown on the screen.

Why do you think you need to do this?

Copy link
Collaborator Author

@Decodetalkers Decodetalkers Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the scale is int, I will not resize it, but when it is fractal, I will resize it manully. Now on your screen it will not be blur anymore. The image will only be blur when the scale is set as 1.2, then I will resize it and make it a little blur, then it can be used to select area. this place it is just used to show the image and let user to select area

Comment on lines -69 to -77
let image_buffer = if let Some(slurp_region) = cli.slurp {
let slurp_region = slurp_region.clone();
let image_buffer = if cli.slurp {
wayshot_conn.screenshot_freeze(
Box::new(move || {
|| -> Result<LogicalRegion> {
let slurp_output = Command::new("slurp")
.args(slurp_region.split(' '))
.output()?
.stdout;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this behaviour? This is removing the ability to pass arguments to slurp.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slurp only accept '-d' to select screen, but -d cannot be passed into, and it is useless

Copy link
Collaborator Author

@Decodetalkers Decodetalkers Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
all the argument is like -d -w, how can you pass the argument into it? I have try send it , but it can never be received

image

like such kind of error

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo run -- --slurp '\-d'
this command will work

but '-d' will never work

and wrong use of slurp, slurp only accept args like `-d`, it do not
accept region, just run -d is enough
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants