Handle layer's negative top or left #46
Conversation
|
Hmm. the problem is very complex here.
|
|
@chinedufn Here are my suggestion,
|
|
I put the change that |
namse
changed the title
| if pixel_left < layer.layer_properties.layer_left as usize | ||
| || pixel_left > layer.layer_properties.layer_right as usize | ||
| || pixel_top < layer.layer_properties.layer_top as usize | ||
| || pixel_top > layer.layer_properties.layer_bottom as usize |
There was a problem hiding this comment.
layer_left, right, top, bottom could be negative, so pixel_left and top should be casted from usize to i32, not casting i32 to usize.
| @@ -203,7 +209,7 @@ fn rle_decompress(bytes: &[u8]) -> Vec<u8> { | |||
| } else { | |||
| let repeat = 1 - header; | |||
| let byte = cursor.read_1()[0]; | |||
| for _ in 0..repeat as usize { | |||
| for _ in 0..repeat { | |||
There was a problem hiding this comment.
I removed this kind of meaningless as usize codes.
| @@ -32,16 +32,16 @@ pub enum ImageDataSectionError { | |||
| #[derive(Debug)] | |||
| pub struct ImageDataSection { | |||
| /// The compression method for the image. | |||
| pub(in crate) compression: PsdChannelCompression, | |||
| pub(crate) compression: PsdChannelCompression, | |||
There was a problem hiding this comment.
This changed by rustfmt and rust-analzyer automatically.
| @@ -161,7 +161,7 @@ impl ImageDataSection { | |||
| let (red_start, red_end) = | |||
| (channel_data_start, channel_data_start + red_byte_count); | |||
|
|
|||
| let red = bytes[red_start as usize..red_end as usize].into(); | |||
| let red = bytes[red_start..red_end].into(); | |||
There was a problem hiding this comment.
these guys are already usize.
| let left_in_layer = idx as u16 % self.width(); | ||
| let left_in_psd = self.layer_properties.layer_left + left_in_layer as i32; | ||
|
|
||
| let top_in_psd = idx / self.width() as usize + self.layer_properties.layer_top as usize; | ||
| let top_in_layer = idx as u16 / self.width(); | ||
| let top_in_psd = self.layer_properties.layer_top + top_in_layer as i32; |
There was a problem hiding this comment.
I casted unsigned integer to signed integer,
previously this casted signed integer to unsigned integer, which can occur overflow.
|
Thanks for working on this! Will review this over the weekend. I haven't looked at the code yet, but if you haven't already please add test(s) for the things that you fixed. These tests should have failed before your fixes and pass after your fixes. |
|
I see, I have a psd file that makes error with previous code. Give me some time to put test. :) |
There was a problem hiding this comment.
Thanks! Just need to land @kevzettler 's tests then I can merge this.
|
@namse @chinedufn I have created a new PR at #52 which contains all the changes from this PR with the test cases. We can close this in favor of #52 |
This PR is a extension of: #46 This PR includes all the code from #46 and additionally has tests and README updates Co-authored-by: namse <[email protected]>
|
Added a commit with both of you as co authors a57f32f |
closes #45
Other little code changes by rust-analyzer and rustfmt automatically.