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] DjVu gamma correction #921

Merged
merged 3 commits into from
Jun 10, 2019
Merged

[fix] DjVu gamma correction #921

merged 3 commits into from
Jun 10, 2019

Conversation

Frenzie
Copy link
Member

@Frenzie Frenzie commented Jun 10, 2019

The existing implementation only worked in 8-bit mode. By using the ddjvu_format_set_gamma() function that comes with djvulibre it suddenly and attractively becomes Not Our Problem™.

I noticed this after doing a quick experiment with a custom power function #ifdef __ANDROID_API__ like in https://stackoverflow.com/a/19488271/2470572

(For example, if Android define a custom power function, else #define power(x, y) pow(x, y).)

I believe the proper way to go about it would probably be to ignore document-specific implementations and to do it in the blitbuffer, but this is quicker to implement.

As a side effect, this fixes koreader/koreader#3493.

The existing implementation only worked in 8-bit mode. By using the `ddjvu_format_set_gamma()` function that comes with djvulibre it suddenly and attractively becomes Not Our Problem™.

I noticed this after doing a quick experiment with a custom power function `#ifdef __ANDROID_API__` like in https://stackoverflow.com/a/19488271/2470572

(For example, if Android define a custom power function, else `#define power(x, y) pow(x, y)`.)

I believe the proper way to go about it would probably be to ignore document-specific implementations and to do it in the blitbuffer, but this is quicker.

As a side effect, this fixes koreader/koreader#3493.
@Frenzie Frenzie added the bug label Jun 10, 2019
@Frenzie Frenzie requested review from NiLuJe and poire-z June 10, 2019 10:22
@Frenzie
Copy link
Member Author

Frenzie commented Jun 10, 2019

Illustration of the problem (because pixels are 24 bits here instead of 8):

Screenshot_2019-06-10_12-24-14

@@ -654,6 +656,20 @@ static int drawPage(lua_State *L) {
BlitBuffer *bb = (BlitBuffer*) lua_topointer(L, 3);
ddjvu_render_mode_t djvu_render_mode = (int) luaL_checkint(L, 6);
ddjvu_rect_t pagerect, renderrect;
// map KOReader gamma to djvulibre gamma
// djvulibre goes from 0.5 to 5.0
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when you give it < 0.5 or > 5.0 ? which can happen if one hacked koptoptions.lua to extend boundary values.

Which it seems I once did, I have that in my own patch:

                 name_text = S.CONTRAST,
                 buttonprogress = true,
-                values = {1/0.8, 1/1.0, 1/1.5, 1/2.0, 1/3.0, 1/4.0, 1/6.0, 1/9.0},
+                -- for pdf reflowing mode (kopt_contrast):
+                values = {1/0.8, 1/1.0, 1/1.5, 1/2.0, 1/4.0, 1/6.0, 1/10.0, 1/50.0},
                 default_pos = 2,
                 default_value = DKOPTREADER_CONFIG_CONTRAST,
                 event = "GammaUpdate",
-                args = {0.8, 1.0, 1.5, 2.0, 3.0, 4.0, 6.0, 9.0},
-                labels = {0.8, 1.0, 1.5, 2.0, 3.0, 4.0, 6.0, 9.0},
+                -- for pdf non-reflowing mode (mupdf):
+                args =   {0.8, 1.0, 1.5, 2.0, 4.0, 6.0, 10.0, 50.0},
+                labels = {0.8, 1.0, 1.5, 2.0, 4.0, 6.0, 10.0, 50.0},
+                -- see https://github.com/koreader/koreader/issues/1299#issuecomment-65183895
                 name_text_hold_callback = optionsutil.showValues,

Copy link
Contributor

Choose a reason for hiding this comment

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

(I mean: I don't mind having my higher values limited and causing no change from previous values - just want to be sure there could be no crash or strange things.)

Copy link
Member Author

Choose a reason for hiding this comment

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

djvulibre ignores out of bound values.

https://gitlab.com/koreader/djvulibre/blob/9658b01431cd7ff6344d7787f855179e73fe81a7/libdjvu/ddjvuapi.cpp#L2166

(One wonders if it wouldn't make more sense to cap them instead of to ignore them completely, but that aside.)

Copy link
Contributor

Choose a reason for hiding this comment

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

it wouldn't make more sense to cap them instead of to ignore them

I think you should cap them, after your gamma = A - B*gamma s, as they could give out of bound values, so better to have them capped that suddenly getting back with no gamma change.

@Frenzie Frenzie merged commit f5f2793 into koreader:master Jun 10, 2019
@Frenzie Frenzie deleted the djvu-gamma branch June 10, 2019 13:28
Frenzie added a commit to Frenzie/koreader that referenced this pull request Jun 10, 2019
Frenzie added a commit to koreader/koreader that referenced this pull request Jun 10, 2019
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Djvu not working on Android 4.2, cannot locate symbol "pow"
2 participants