feat: multiscale #17
feat: multiscale #17
Conversation
|
Result |
|
Sorry, a file was missed and has not been committed. It has now been committed |
| @@ -16,6 +16,10 @@ def __init__(self, image: SingleImage, hotspot: Tuple[int, int], nominal: int) - | |||
| def __repr__(self) -> str: | |||
| return f'CursorImage(image={self.image!r}, hotspot={self.hotspot!r}, nominal={self.nominal!r})' | |||
|
|
|||
| def scale(self, width: int, height: int) -> None: | |||
| self.image.scale(width, height) | |||
| self.nominal = max(width, height) | |||
There was a problem hiding this comment.
This makes sense when generating new sizes, but I am not yet convinced it actually makes sense in general.
The nominal size is a special field in the Xcursor format that helps in selecting the image to use, but the actual image can be any size you want. There are weird situations with cursors that have a lot of padding around the actual image. In those cases, the outcome might not be desirable, which is why scaling was originally introduced. If we adjust the nominal size, then on the Linux side, a different cursor sprite would be selected, resulting in identical visual outcomes.
| @@ -34,6 +34,10 @@ def main() -> None: | |||
| help='color of the shadow') | |||
| parser.add_argument('--scale', default=None, type=float, | |||
| help='Scale the cursor by the specified factor.') | |||
| parser.add_argument('--multiscale', action='store_true', | |||
| help='Generate multiple sizes for each cursor.') | |||
| parser.add_argument('--multiscale-min', type=int, default=12, | |||
There was a problem hiding this comment.
Why is this default 12 when the array later is starts at 16?
| @@ -48,7 +52,9 @@ def process(file: BinaryIO) -> None: | |||
| print(f'Error occurred while processing {name}:', file=sys.stderr) | |||
| traceback.print_exc() | |||
| else: | |||
| if args.scale: | |||
| if args.multiscale: | |||
| multiscale.generates_frames(cursor=cursor, min_size=args.multiscale_min) | |||
There was a problem hiding this comment.
| multiscale.generates_frames(cursor=cursor, min_size=args.multiscale_min) | |
| multiscale.generates_frames(cursor, min_size=args.multiscale_min) |
| min_size (int): The minimum size to generate. | ||
|
|
||
| Returns: | ||
| List[Cursor]: The generated cursors. |
There was a problem hiding this comment.
This directly contradicts the -> None. In general, I don't like declaring signatures in comments when you can explicitly declare them in the signature. DRY.
| """ | ||
| frames = cursor.frames | ||
| new_frames = [] | ||
| image_size = frames[0].images[0].nominal |
There was a problem hiding this comment.
Shouldn't you pick the biggest image in the frame if you are resizing it? I am pretty sure the code doesn't sort it from biggest to smallest.
| del cursor.frames[:] | ||
| cursor.frames.extend(new_frames) |
There was a problem hiding this comment.
Inefficient, try:
| del cursor.frames[:] | |
| cursor.frames.extend(new_frames) | |
| cursor.frames[:] = new_frames |
There was a problem hiding this comment.
Is this supposed to be a win2xcur feature only?
| new_images = [] | ||
| for cur in frame: | ||
| new_cur = CursorImage(cur.image.clone(), cur.hotspot, cur.nominal) | ||
| new_cur.scale(size, size) |
There was a problem hiding this comment.
What if the image is rectangular? Also why is every image in the frame scaled to the same size?
| new_cur = CursorImage(cur.image.clone(), cur.hotspot, cur.nominal) | ||
| new_cur.scale(size, size) | ||
| new_images.append(new_cur) | ||
| new_frame = CursorFrame(new_images, frame.delay) |
There was a problem hiding this comment.
Why are you looping over all the sizes and repeat each frame for each size? I see why you run into the nominal size problem now. If the nominal size is different in different frames, then Xcursor treats them as separate sizes and not animations. This is a sign of a massive bug in your implementation—the number of frames should be constant despite all your image manipulation.
Implement #16
Changes
--multiscaleto generate multiscale of cursor, and using--multiscale-minto set the min size.Bug at:
win2xcur/win2xcur/scale.py
Lines 9 to 12 in 8e71037
win2xcur/win2xcur/cursor.py
Lines 12 to 14 in 8e71037