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

Add a few RGB-aware set & blend methods to the blitters #1680

Merged
merged 43 commits into from
Aug 29, 2024

Conversation

smasher816
Copy link
Contributor

@smasher816 smasher816 commented Oct 26, 2023

Add new BB_fill_rect_color and BB_blend_rect_color functions taking a ColorRGB32.
Use a new is_rgb flag in Color structs for lua wrapper to decide which BB_ function to call.
If a ColorRGB32 is passed to fillRect/lightenRect then the new path will be taken. Otherwise original Color8A/uint8_t code is called.

No changes should occur for existing code paths. Callers must be patched to provide RGB colors if desired.
Lua blitbuffer implementation has not been changed.


This change is Reviewable

@smasher816
Copy link
Contributor Author

Initial inspiration. koreader/koreader#9024 (comment)

blitbuffer.c Outdated Show resolved Hide resolved
blitbuffer.c Show resolved Hide resolved
blitbuffer.c Show resolved Hide resolved
ffi/blitbuffer.lua Outdated Show resolved Hide resolved
@smasher816
Copy link
Contributor Author

Unrelated, but all of a sudden after pushing to forks I am unable to recompile. Any pointers would be helpful since I haven't touched 3rd-party deps.
I would like to tests my fixes on-device before updating the MR.

dewarp2.c:144:8: error: call to undeclared function 'dewarpBuildPageModel_ex'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
return(dewarpBuildPageModel_ex(dew,debugfile,2));
       ^
dewarp2.c:144:8: note: did you mean 'dewarpBuildPageModel'?
dewarp2.c:141:1: note: 'dewarpBuildPageModel' declared here
dewarpBuildPageModel(L_DEWARP    *dew,
^
dewarp2.c:230:9: error: call to undeclared function 'dewarpFindVertDisparity_ex'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    if (dewarpFindVertDisparity_ex(dew, ptaa2, 0, fit_order) != 0) {
        ^
dewarp2.c:230:9: note: did you mean 'dewarpFindVertDisparity'?
./allheaders.h:654:25: note: 'dewarpFindVertDisparity' declared here
LEPT_DLL extern l_int32 dewarpFindVertDisparity ( L_DEWARP *dew, PTAA *ptaa, l_int32 rotflag );
                        ^
dewarp2.c:301:8: error: call to undeclared function 'dewarpFindVertDisparity_ex'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
return(dewarpFindVertDisparity_ex(dew,ptaa,rotflag,2));
       ^
dewarp2.c:1411:8: error: call to undeclared function 'dewarpBuildLineModel_ex'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
return(dewarpBuildLineModel_ex(dew,opensize,debugfile,2));
       ^
4 errors generated.
make[4]: *** [Makefile:823: dewarp2.lo] Error 1
make[4]: *** Waiting for unfinished jobs....
make[4]: Leaving directory '/home/arch/git/koreader/base/thirdparty/leptonica/build/aarch64-unknown-linux-android21/leptonica-prefix/src/leptonica/src'
make[3]: *** [Makefile:470: install-recursive] Error 1
make[3]: Leaving directory '/home/arch/git/koreader/base/thirdparty/leptonica/build/aarch64-unknown-linux-android21/leptonica-prefix/src/leptonica'
make[2]: *** [Makefile:95: liblept.so] Error 2
make[2]: Leaving directory '/home/arch/git/koreader/base/thirdparty/libk2pdfopt/build/aarch64-unknown-linux-android21/libk2pdfopt-prefix/src/libk2pdfopt'
ninja: build stopped: subcommand failed.
make[1]: *** [Makefile.third:372: build/aarch64-unknown-linux-android21/libs/libk2pdfopt.so.2] Error 1

@NiLuJe
Copy link
Member

NiLuJe commented Oct 27, 2023

Unrelated

Make sure your submodules are up-to-date and point to the right place (both base's and base itself), then try a make dist-clean?

@smasher816
Copy link
Contributor Author

Fixed the above. A fresh clone also had issues so I figured out it was something with the machine and not the koreader repo.
After looking into env I found the following unexpectedly set.

profile.d/android-ndk.sh
2:export ANDROID_NDK=/opt/android-ndk
4:export ANDROID_NDK_HOME=/opt/android-ndk
6:export ANDROID_NDK_ROOT=/opt/android-ndk

It looks like installing a system package added those variables by default.

$ pacman -Qo /etc/profile.d/android-ndk.sh
/etc/profile.d/android-ndk.sh is owned by android-ndk r26.b-1

Unsetting the above fixed everything and now I can compile again. I'll work on resolving your comments.

@smasher816
Copy link
Contributor Author

New revision should have support for other screentypes and for real color blending (stacking multiple highlights makes the color get stronger and stronger).

@smasher816
Copy link
Contributor Author

Newest revision supports color pdf highlights. If new bb_color arg is not passed then the existing defaults are used.

blitbuffer.c Outdated Show resolved Hide resolved
Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

I've pushed some minor simplifications, but have NOT tested them, I've just checked that it built (and was generating similar machine code before actual logic deviations were implemented ;p).

Reviewed 1 of 3 files at r2, 3 of 3 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @smasher816)

Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @smasher816)

@poire-z
Copy link
Contributor

poire-z commented Nov 17, 2023

Is this ok? (I mean: ready to merge?)

@NiLuJe
Copy link
Member

NiLuJe commented Nov 17, 2023

Needs testing ;).

@NiLuJe
Copy link
Member

NiLuJe commented Jan 21, 2024

@poire-z: Because I'm super lazy, If you have a random test file with colored text on hand, that could come in handy. Thanks ;).

@poire-z
Copy link
Contributor

poire-z commented Jan 21, 2024

Because I'm super lazy,

I know :)
Here's 2 of my test.html files where I tested various text & bg colors: test-color-stuff.zip
Any Wikipedia saved as EPUB will probably have various kinds of colored images if you need more.

@NiLuJe NiLuJe changed the title Add initial RGB painting support to C blitbuffer Add a few RGB-aware set & blend methods to the blitters Jan 22, 2024
smasher816 and others added 4 commits August 25, 2024 19:48
And re-implement RGB32 fast path
(Jokes aside, we tend to prefer that syntatcit sugar (and string
methods) as opposed to calling string.fn(obj) explicitly).
Requires a few bendRGB32 shims for Color <= 8A, as the usual blend
methods assume Color8 input.
Which is the one we actually care about, as we don't actually have any
colorblitFromRGB32 callers ;).
To deal with highlights on sw nightmode...
Implement a grayscale and a color one, like other setters

Instead of implementing a full array of per-color type blend methods
And round the component values to 16 shades per component.
We often need a copy anyway, so take the opportunity to make this safer
;)
This allows us to fairly easily make gray behave as our legacy grayscale
highlights, honoring the user's opacity value.
Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r8, 1 of 1 files at r9, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @smasher816)

Apparently, 8 wasn't enough, the highlight style dialog was still cutoff
:/
dithering.

So, rejig that so we can tweak that on a per-device basis, too.
@NiLuJe NiLuJe marked this pull request as ready for review August 26, 2024 22:46
@NiLuJe
Copy link
Member

NiLuJe commented Aug 26, 2024

Hey, look, I've finally gotten around to finish this before the PR's first birthday ;p.

(I basically just want to test a few color changes tomorrow, but other than that, it's ready).

@NiLuJe
Copy link
Member

NiLuJe commented Aug 27, 2024

ChangeLog:

  • BlitBuffer: Added a fill_rect variant that takes an RGB32 color as input
  • BlitBuffer: Added a couple of blend OVER & blend MUL methods to handle color highlights
  • BlitBuffer: Added a color_blit_from variant that takes an RGB32 color as input
  • C BlitBuffer: Fix a few minor compiler warnings
  • BlitBuffer: Rename the awfully named dimRect & lightenRect methods to lightenRect & darkenRect, respectively. Because they did the opposite of what it said on the tin. Added more comments explaining the results to avoid further confusion.
  • BlitBuffer: Defined a set of highlight colors as BB.HIGHLIGHT_COLORS (user patchable), most of them having a more pastel variant because those look slightly less terrible on Kaleido panels.
  • BlitBuffer: Added a few miscellaneous methods to ease working with this in front.
  • Kobo MTK: Increased the alignment constraint, because the HL color popup was getting cut off.
  • MuPDF: Support color annotations in addMarkupAnnotation

Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r10, 1 of 1 files at r11, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @smasher816)

@NiLuJe
Copy link
Member

NiLuJe commented Aug 27, 2024

Commit history will be archived in my color-highlights branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants