apps/kiv: loading icons from shared memory, libs/libimg: crop function #573

2026-07-14T15:02:37Z

IgorA commented

2026-07-14 15:02:37 +00:00

add 3 icons: save as, edit in other app, crop
libimg: add new function 'img_crop'

add 3 icons: save as, edit in other app, crop libimg: add new function 'img_crop'

Burer requested changes

2026-07-14 16:05:09 +00:00

Dismissed

programs/develop/libraries/libs-dev/libimg/libimg.asm Outdated

						
				@@ -1033,0 +1033,4 @@

				;;============================================================================;;

				proc img.crop _img, _left, _top, _width, _height

				;;----------------------------------------------------------------------------;;

				;? Сrops the image according to the specified coordinates                     ;;

Burer commented

2026-07-14 16:05:02 +00:00

First letter is Cyrillic С, must be Latin C.

Burer marked this conversation as resolved

IgorA added the Lang/FASM label 2026-07-14 20:32:49 +00:00

Burer requested changes

2026-07-15 06:40:47 +00:00

Dismissed

Burer left a comment

🔴 img.crop, bpp24 path: rep movsb with an undefined ecx
When b_lost == 0 (_width == Image.Width, vertical-only crop) the fast path runs rep movsb without loading ecx. The intended count (_width*_height*3) was in ecx before invoke mem.alloc, which may clobber it. The bpp8i sibling reloads ecx (line 1092); bpp24 doesn't.
🟡 img.crop silently no-ops on bpp32 and bpp8g
Only bpp8i and bpp24 are handled; other types fall to .fail and return 0. image_crop treats 0 as "do nothing", so cropping a 32-bit (alpha) image does nothing. Consider implementing crop for them or adding some notification for users.
🟡 image_save_as calls free_directory
Saving a file frees directory_ptr, the cached folder listing used for next/prev navigation.
Do we really need to clean cache directory on saving file?

- [x] **🔴 `img.crop`, bpp24 path: `rep movsb` with an undefined `ecx`** When `b_lost == 0` (`_width == Image.Width`, vertical-only crop) the fast path runs `rep movsb` without loading `ecx`. The intended count (`_width*_height*3`) was in `ecx` before `invoke mem.alloc`, which may clobber it. The bpp8i sibling reloads `ecx` (line 1092); bpp24 doesn't. - [x] **🟡 `img.crop` silently no-ops on bpp32 and bpp8g** Only bpp8i and bpp24 are handled; other types fall to `.fail` and return 0. `image_crop` treats 0 as "do nothing", so cropping a 32-bit (alpha) image does nothing. Consider implementing crop for them or adding some notification for users. - [x] **🟡 `image_save_as` calls `free_directory`** Saving a file frees `directory_ptr`, the cached folder listing used for next/prev navigation. Do we really need to clean cache directory on saving file?

Burer changed title from ~~Kiv: loading icons from shared memory,~~ to apps/kiv: loading icons from shared memory,

2026-07-15 06:42:26 +00:00

Burer added the

labels 2026-07-15 06:42:55 +00:00

Burer requested review from Developers 2026-07-15 06:43:00 +00:00

Burer requested review from Owners 2026-07-15 06:43:01 +00:00

Burer changed title from ~~apps/kiv: loading icons from shared memory,~~ to apps/kiv: loading icons from shared memory, libs/libimg: crop function

2026-07-15 06:43:18 +00:00

IgorA commented

2026-07-15 08:56:21 +00:00

Saving a file frees directory_ptr, the cached folder listing used for next/prev navigation.
Do we really need to clean cache directory on saving file?

If I open a file from a folder and then save it to a different one, the program crashes when I click the buttons to open the next or previous image. However, the program does not crash after adding a call to the directory_ptr function. I left the question marks there because, when clicking the next or previous image buttons, the program opens images from the original folder. Ideally, the current folder should be updated after saving, but I don't know which functions to use for that.

> Saving a file frees `directory_ptr`, the cached folder listing used for next/prev navigation. > Do we really need to clean cache directory on saving file? If I open a file from a folder and then save it to a different one, the program crashes when I click the buttons to open the next or previous image. However, the program does not crash after adding a call to the `directory_ptr` function. I left the question marks there because, when clicking the next or previous image buttons, the program opens images from the original folder. Ideally, the current folder should be updated after saving, but I don't know which functions to use for that.

👍 1

Burer requested changes

2026-07-16 04:53:54 +00:00

Dismissed

Burer left a comment

🟡 image_save_as: get_libimg_format returns 0 for raw, so the raw branch is entered, but it then does cmp [Image.Type], Image.bpp24; jnz @f. For a non-24-bit image (e.g. a 32-bit PNG with alpha) it falls through to cmp eax, 1; jl .end_f with eax = 0 and returns - nothing is written and no error is shown.
Other formats (png/bmp/…) go through img.encode and handle 32-bit fine; only raw is affected. Consider either converting to 24-bit before the raw dump or reject the save with a message.

- [x] 🟡 `image_save_as`: `get_libimg_format` returns 0 for `raw`, so the raw branch is entered, but it then does `cmp [Image.Type], Image.bpp24; jnz @f`. For a non-24-bit image (e.g. a 32-bit PNG with alpha) it falls through to `cmp eax, 1; jl .end_f` with `eax = 0` and returns - nothing is written and no error is shown. Other formats (png/bmp/…) go through `img.encode` and handle 32-bit fine; only raw is affected. Consider either converting to 24-bit before the raw dump or reject the save with a message.

Leency commented

2026-07-16 16:54:01 +00:00

Asterisk is in more usual position now

image.png

3.3 KiB

Leency commented

2026-07-16 16:56:01 +00:00

I like that selection goes througt all canvas. This is handy.

~~Unfortunatelly, aftifacts appear from time to time.~~
Fixed.

I like that selection goes througt all canvas. This is handy. ~~Unfortunatelly, aftifacts appear from time to time.~~ Fixed.

Leency requested changes

2026-07-16 17:02:41 +00:00

Dismissed

programs/media/kiv/kiv.asm Outdated

						
				@@ -1296,2 +2027,3 @@

				        add     edx, ((5+25*1) SHL 16)+0

				;        mcall     , buttons+scalebtn  *20

				        mcall     , buttons+bgrbtn    *ICON_SIZE

				        add     edx, ((0+25*1) SHL 16)+0

Leency commented

2026-07-16 17:02:22 +00:00

Could you please replace "25" by a constant that is easy to change? I would prefere a little more horisontal space around icons. We can afford it as the minimal window width is less than 400px now.

Probably +1 px around each icon and +2 px around section separator.

Could you please replace "25" by a constant that is easy to change? I would prefere a little more horisontal space around icons. We can afford it as the minimal window width is less than 400px now. Probably +1 px around each icon and +2 px around section separator.

Leency marked this conversation as resolved

Burer force-pushed kiv-update from c2a20ccdbc to c78f015508

2026-07-18 10:55:32 +00:00

Compare

Burer requested changes

2026-07-18 12:07:41 +00:00

Dismissed

Burer left a comment

1. Saving as .raw permanently converts the open image to 24-bit

invoke  img.convert, ebx, 0, Image.bpp24, 0, 0
mov     [orig_image], eax
mov     [cur_image], eax
mov     [cur_frame], eax
invoke  img.destroy, ebx

Nothing restores the original, so a 32-bit image loses its alpha for the rest of the session and a later "save as PNG" writes 24-bit data.

2. The scaled image is leaked on that path

In LIBIMG_SCALE_FIT_MIN mode cur_image is a separate object from img.scale. Overwriting [cur_image] above without destroying it strands that buffer: the next draw_view sees cur_image == orig_image, skips its img.destroy, and allocates a new one.

I believe, using a temporary image for the conversion fixes both of these problems.

**1. Saving as `.raw` permanently converts the open image to 24-bit** ```asm invoke img.convert, ebx, 0, Image.bpp24, 0, 0 mov [orig_image], eax mov [cur_image], eax mov [cur_frame], eax invoke img.destroy, ebx ``` Nothing restores the original, so a 32-bit image loses its alpha for the rest of the session and a later "save as PNG" writes 24-bit data. **2. The scaled image is leaked on that path** In `LIBIMG_SCALE_FIT_MIN` mode `cur_image` is a separate object from `img.scale`. Overwriting `[cur_image]` above without destroying it strands that buffer: the next `draw_view` sees `cur_image == orig_image`, skips its `img.destroy`, and allocates a new one. I believe, using a temporary image for the conversion fixes both of these problems.

Leency approved these changes

2026-07-21 07:49:05 +00:00

Burer approved these changes

2026-07-21 07:50:34 +00:00

Burer added 13 commits 2026-07-21 07:52:10 +00:00

Kiv: loading icons from shared memory, 9cf5ce7d4e

add 3 icons: save as, edit in other app, crop
libimg: add new function 'img_crop'

libimg fix Cyrillic C efb5f93c46

libimg fix function crop 77184126fe

correct the allocation calculation 8d2f0bfca1

non-24-bit images are converted before being saved as raw 658ca5aac3

kiv: better asterisk position fe1c6019d5

kiv: fix selection artifacts, fix bits crop in fullscreen a4db322efa

change buttons size to 25 px 62c926523b

fix size fullscreen buttons a2435de5de

kiv: use system colors de2791a6b4

saving an 8 bit raw image as 24 bit image from a temporary image 87aee08956

fix Rhimad avatar :) d992e2729b

kiv: free the raw buffer when the 24-bit conversion fails

Test PR / Build (en_US) (pull_request) Successful in 1m59s

Details

Test PR / Build (ru_RU) (pull_request) Successful in 2m4s

Details

Test PR / Build (es_ES) (pull_request) Successful in 2m7s

Details

8dfd179568

encode_raw_rgb allocated raw_data before calling img.convert, but on a
convert failure it jumped straight to .fail, which only zeroed the return
registers - the buffer was never released. Saving a non-24-bit image as
.raw under memory pressure therefore leaked 3*w*h bytes per attempt.

Split the exit: the alloc-failure path still goes to .fail (nothing to
free there), the convert-failure path goes to a new .fail_free that frees
raw_data first and then falls through to .fail.

Burer force-pushed kiv-update from f52100781a to 8dfd179568

2026-07-21 07:52:10 +00:00

Compare

Burer merged commit 14162d4139 into main

2026-07-21 07:55:53 +00:00

Burer deleted branch kiv-update

2026-07-21 07:55:54 +00:00

Egor00f referenced this issue from a commit

2026-08-27 10:35:46 +00:00

apps/kiv: loading icons from shared memory, libs/libimg: crop function (#573)

Sign in to join this conversation.

No Reviewers

Leency

Burer

KolibriOS/Developers

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#573