[PATCH v10 0/1] MR5904: Improve color mode and paper source detection
-- v10: sane.ds: Improve color mode and paper source detection https://gitlab.winehq.org/wine/wine/-/merge_requests/5904
From: Ilia Docin <ilya.docin(a)contentai.ru> --- dlls/sane.ds/capability.c | 152 ++++++++++++++++++++++++-------------- dlls/sane.ds/options.c | 39 ++++++---- dlls/sane.ds/sane_i.h | 2 +- 3 files changed, 124 insertions(+), 69 deletions(-) diff --git a/dlls/sane.ds/capability.c b/dlls/sane.ds/capability.c index 555575c45a4..9106f05d076 100644 --- a/dlls/sane.ds/capability.c +++ b/dlls/sane.ds/capability.c @@ -277,47 +277,32 @@ static TW_UINT16 SANE_CAPXferCount (pTW_CAPABILITY pCapability, TW_UINT16 action return twCC; } -static TW_UINT16 set_color_mode(TW_UINT32 pixeltype) +static TW_UINT16 set_option_value(const char* option_name, char* option_value) { - TW_UINT16 twCC = TWCC_BADCAP; + TW_UINT16 twCC = TWCC_BADVALUE; BOOL reload = FALSE; - int i; - const char * const * modes; - /* most of the values are taken from https://gitlab.gnome.org/GNOME/simple-scan/-/blob/master/src/scanner.vala */ - static const char * const gray[] = {"Gray", "Grayscale", "True Gray", "8-bit Grayscale", "Grayscale - 256 Levels", "gray", - 0}; - static const char * const rgb[] = {"Color", "24bit Color[Fast]", "24bit Color", - "24 bit Color", "Color - 16 Million Colors", 0}; - static const char * const bw[] = {"Lineart", "LineArt", "Black & White", "Binary", "Thresholded", "1-bit Black & White", - "Black and White - Line Art", "Black and White - Halftone", "Monochrome", 0}; - switch (pixeltype) + if (*option_value) { - case TWPT_GRAY: - modes = gray; - break; - case TWPT_RGB: - modes = rgb; - break; - case TWPT_BW: - modes = bw; - break; - default: - ERR("Unsupported pixeltype %lu\n", pixeltype); - return TWCC_BADVALUE; - break; - } - TRACE("Setting pixeltype to %lu\n", pixeltype); - for(i=0; modes[i]; ++i) - { - twCC = sane_option_set_str("mode", (char*)modes[i], &reload); + twCC = sane_option_set_str(option_name, option_value, &reload); if (twCC == TWCC_SUCCESS) { if (reload) get_sane_params(&activeDS.frame_params); - break; } } return twCC; } +static TW_UINT16 find_value_pos(const char* value, const char* values, TW_UINT16 buf_len, TW_UINT16 buf_count) +{ + TW_UINT16 index; + for (index=0; index<buf_count; ++index) + { + if (!strncmp(value, values, buf_len)) + return index; + values += buf_len; + } + return buf_count; +} + /* ICAP_PIXELTYPE */ static TW_UINT16 SANE_ICAPPixelType (pTW_CAPABILITY pCapability, TW_UINT16 action) { @@ -326,16 +311,39 @@ static TW_UINT16 SANE_ICAPPixelType (pTW_CAPABILITY pCapability, TW_UINT16 actio int possible_value_count; TW_UINT32 val; TW_UINT16 current_pixeltype = TWPT_BW; + enum { buf_len = 64, buf_count = 3 }; + char color_modes[buf_len * buf_count] = {'\0'}, current_mode[buf_len] = {'\0'}, *output = 0; + /* most of the values are taken from https://gitlab.gnome.org/GNOME/simple-scan/-/blob/master/src/scanner.vala */ + static const WCHAR* bw[] = {L"Lineart", L"LineArt", L"Black & White", L"Binary", L"Thresholded", + L"1-bit Black & White", L"Black and White - Line Art", L"Black and White - Halftone", L"Monochrome", L"bw", 0}; + static const WCHAR* gray[] = {L"Gray", L"Grayscale", L"True Gray", L"8-bit Grayscale", L"Grayscale - 256 Levels", + L"gray", 0}; + static const WCHAR* rgb[] = {L"Color", L"24bit Color[Fast]", L"24bit Color", L"24 bit Color", + L"Color - 16 Million Colors", L"color", 0}; + static const WCHAR* const* filter[] = {bw, gray, rgb, 0}; TRACE("ICAP_PIXELTYPE\n"); + twCC = sane_option_probe_str("mode", filter, color_modes, buf_len); + if (twCC != TWCC_SUCCESS) + { + ERR("Unable to retrieve modes from sane, ICAP_PIXELTYPE unsupported\n"); + return twCC; + } - twCC = sane_option_probe_mode(¤t_pixeltype, possible_values, &possible_value_count); + twCC = sane_option_get_str("mode", current_mode, buf_len); if (twCC != TWCC_SUCCESS) { - ERR("Unable to retrieve mode from sane, ICAP_PIXELTYPE unsupported\n"); + ERR("Unable to retrieve current mode from sane, ICAP_PIXELTYPE unsupported\n"); return twCC; } + current_pixeltype = find_value_pos(current_mode, color_modes, buf_len, buf_count); + if (current_pixeltype == buf_count) + { + ERR("Wrong current mode value, ICAP_PIXELTYPE unsupported\n"); + twCC = TWCC_BADVALUE; + return twCC; + } /* Sane does not support a concept of a default mode, so we simply cache * the first mode we find */ if (! activeDS.PixelTypeSet) @@ -352,17 +360,28 @@ static TW_UINT16 SANE_ICAPPixelType (pTW_CAPABILITY pCapability, TW_UINT16 actio break; case MSG_GET: + possible_value_count = 0; + for (val=TWPT_BW; val<=TWPT_RGB; ++val) + { + if (*(color_modes + val * buf_len)) + possible_values[possible_value_count++] = val; + } twCC = msg_get_enum(pCapability, possible_values, possible_value_count, TWTY_UINT16, current_pixeltype, activeDS.defaultPixelType); break; case MSG_SET: twCC = msg_set(pCapability, &val); - if (twCC == TWCC_SUCCESS) + if ((twCC == TWCC_SUCCESS) && (val < buf_count)) { - TRACE("Setting pixeltype to %ld\n", val); - twCC = set_color_mode(val); + output = color_modes + val * buf_len; + TRACE("Setting pixeltype to %lu: %s\n", val, output); + twCC = set_option_value("mode", output); + if (twCC != TWCC_SUCCESS) + ERR("Unable to set pixeltype to %lu: %s\n", val, output); } + else + twCC = TWCC_BADVALUE; break; case MSG_GETDEFAULT: @@ -370,7 +389,7 @@ static TW_UINT16 SANE_ICAPPixelType (pTW_CAPABILITY pCapability, TW_UINT16 actio break; case MSG_RESET: - twCC = set_color_mode(activeDS.defaultPixelType); + twCC = set_option_value("mode", color_modes + activeDS.defaultPixelType * buf_len); if (twCC == TWCC_SUCCESS) current_pixeltype = activeDS.defaultPixelType; else @@ -933,17 +952,39 @@ static TW_UINT16 SANE_CAPFeederEnabled (pTW_CAPABILITY pCapability, TW_UINT16 ac TW_UINT16 twCC = TWCC_BADCAP; TW_UINT32 val; TW_BOOL enabled; - char source[64]; + enum { buf_len = 64, buf_count = 2 }; + char paper_sources[buf_len * buf_count] = {'\0'}, current_source[buf_len] = {'\0'}, *output = 0; + /* most of the values are taken from https://gitlab.gnome.org/GNOME/simple-scan/-/blob/master/src/scanner.vala */ + static const WCHAR* flatbed[] = {L"Flatbed", L"FlatBed", L"Platen", L"Normal", L"Document Table", 0}; + static const WCHAR* autofeeder[] = {L"Auto", L"ADF", L"ADF Front", L"ADF Back", L"adf", + L"Automatic Document Feeder", L"Automatic Document Feeder(centrally aligned)", + L"Automatic Document Feeder(center aligned)", L"Automatic Document Feeder(left aligned)", + L"ADF Simplex" L"DP", 0}; + static const WCHAR* const* filter[] = {flatbed, autofeeder, 0}; TRACE("CAP_FEEDERENABLED\n"); + twCC = sane_option_probe_str("source", filter, paper_sources, buf_len); + if (twCC != TWCC_SUCCESS) + { + ERR("Unable to retrieve paper sources from sane, CAP_FEEDERENABLED unsupported\n"); + return twCC; + } - if (sane_option_get_str("source", source, sizeof(source)) != TWCC_SUCCESS) - return TWCC_BADCAP; + twCC = sane_option_get_str("source", current_source, buf_len); + if (twCC != TWCC_SUCCESS) + { + ERR("Unable to retrieve current paper source from sane, CAP_FEEDERENABLED unsupported\n"); + return twCC; + } - if (strcmp(source, "Auto") == 0 || strcmp(source, "ADF") == 0) - enabled = TRUE; - else - enabled = FALSE; + val = find_value_pos(current_source, paper_sources, buf_len, buf_count); + if (val == buf_count) + { + ERR("Wrong current paper source value, CAP_FEEDERENABLED unsupported\n"); + twCC = TWCC_BADVALUE; + return twCC; + } + enabled = val > 0; switch (action) { @@ -958,16 +999,16 @@ static TW_UINT16 SANE_CAPFeederEnabled (pTW_CAPABILITY pCapability, TW_UINT16 ac case MSG_SET: twCC = msg_set(pCapability, &val); - if (twCC == TWCC_SUCCESS) + if ((twCC == TWCC_SUCCESS) && (val < buf_count)) { - strcpy(source, "ADF"); - twCC = sane_option_set_str("source", source, NULL); + output = paper_sources + val * buf_len; + TRACE("Setting paper source to %lu: %s\n", val, output); + twCC = set_option_value("source", output); if (twCC != TWCC_SUCCESS) - { - strcpy(source, "Auto"); - twCC = sane_option_set_str("source", source, NULL); - } + ERR("Unable to set paper source to %lu: %s\n", val, output); } + else + twCC = TWCC_BADVALUE; break; case MSG_GETDEFAULT: @@ -975,9 +1016,12 @@ static TW_UINT16 SANE_CAPFeederEnabled (pTW_CAPABILITY pCapability, TW_UINT16 ac break; case MSG_RESET: - strcpy(source, "Auto"); - if (sane_option_set_str("source", source, NULL) == TWCC_SUCCESS) - enabled = TRUE; + val = *(paper_sources + buf_len) ? 1 : 0; // set to Auto/ADF if it's supported + output = paper_sources + val * buf_len; + TRACE("Resetting paper source to %lu: %s\n", val, output); + twCC = set_option_value("source", output); + if (twCC != TWCC_SUCCESS) + ERR("Unable to reset paper source to %lu: %s\n", val, output); /* .. fall through intentional .. */ case MSG_GETCURRENT: diff --git a/dlls/sane.ds/options.c b/dlls/sane.ds/options.c index 1d39844a770..fd80efb4145 100644 --- a/dlls/sane.ds/options.c +++ b/dlls/sane.ds/options.c @@ -112,30 +112,41 @@ TW_UINT16 sane_option_probe_resolution(const char *option_name, struct option_de return sane_find_option(option_name, TYPE_INT, opt); } -TW_UINT16 sane_option_probe_mode(TW_UINT16 *current, TW_UINT32 *choices, int *count) +static TW_UINT32 sane_categorize_value(const WCHAR* value, const WCHAR* const* filter[], char* categories, int buf_len) +{ + TW_UINT32 i, j; + for(i=0; filter[i]; ++i) + { + if (!*categories) + { + for(j=0; filter[i][j]; ++j) + { + if (!wcscmp(value, filter[i][j])) + { + wcstombs(categories, value, buf_len); + return i; + } + } + } + categories += buf_len; + } + return 0; +} + +TW_UINT16 sane_option_probe_str(const char* option_name, const WCHAR* const* filter[], char* opt_values, int buf_len) { WCHAR *p; - char buffer[256]; struct option_descriptor opt; - TW_UINT16 rc = sane_find_option("mode", TYPE_STRING, &opt); + TW_UINT16 rc = sane_find_option(option_name, TYPE_STRING, &opt); if (rc != TWCC_SUCCESS) return rc; - if (opt.size > sizeof(buffer)) return TWCC_BADVALUE; - rc = sane_option_get_value( opt.optno, buffer ); - if (rc != TWCC_SUCCESS) return rc; - - if (!strcmp( buffer, "Lineart" )) *current = TWPT_BW; - else if (!strcmp( buffer, "Color" )) *current = TWPT_RGB; - else if (!strncmp( buffer, "Gray", 4 )) *current = TWPT_GRAY; + if (opt.size > buf_len) return TWCC_BADVALUE; - *count = 0; if (opt.constraint_type == CONSTRAINT_STRING_LIST) { for (p = opt.constraint.strings; *p; p += lstrlenW(p) + 1) { - if (!wcscmp( p, L"Lineart" )) choices[(*count)++] = TWPT_BW; - else if (!wcscmp( p, L"Color" )) choices[(*count)++] = TWPT_RGB; - else if (!wcsncmp( p, L"Gray", 4 )) choices[(*count)++] = TWPT_GRAY; + sane_categorize_value(p, filter, opt_values, buf_len); } } return rc; diff --git a/dlls/sane.ds/sane_i.h b/dlls/sane.ds/sane_i.h index 47c5d69c82e..e35ed77c935 100644 --- a/dlls/sane.ds/sane_i.h +++ b/dlls/sane.ds/sane_i.h @@ -201,7 +201,7 @@ TW_UINT16 sane_option_set_int( const char *option_name, int val, BOOL *reload ); TW_UINT16 sane_option_get_str( const char *option_name, char *val, int len ); TW_UINT16 sane_option_set_str( const char *option_name, char *val, BOOL *reload ); TW_UINT16 sane_option_probe_resolution( const char *option_name, struct option_descriptor *opt ); -TW_UINT16 sane_option_probe_mode(TW_UINT16 *current, TW_UINT32 *choices, int *count); +TW_UINT16 sane_option_probe_str( const char* option_name, const WCHAR* const* filter[], char* opt_vals, int buf_len ); TW_UINT16 sane_option_get_bool( const char *option_name, BOOL *val ); TW_UINT16 sane_option_set_bool( const char *option_name, BOOL val ); TW_UINT16 sane_option_get_scan_area( int *tlx, int *tly, int *brx, int *bry ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5904
On Mon Jun 24 16:35:09 2024 +0000, Esme Povirk wrote:
The commits here don't follow the guidelines: https://wiki.winehq.org/Submitting_Patches#Patch_guidelines The "is_settable" code is added and later removed, and `find_value_pos` is introduced broken and later fixed. I think it'd be OK to squash these into one commit. Sure. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5904#note_74239
On Sun Jun 23 04:33:13 2024 +0000, Ilia Docin wrote:
Yea, it was. Scanner might not support "Auto" or ADF sources recommended by TWAIN spec as default values. In that case the command will cause an error. My fix also try to set "Auto" or ADF sources as primary default option, but additionally checks if the modes are supported. If I understand correctly (and there's a lot here that I could be confused about), we're supposed to return the value that this was set to, and this will return `enabled` which is the previous value. So I think we should set `enabled = val` if setting the option succeeds.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5904#note_74240
participants (3)
-
Esme Povirk (＠madewokherd) -
Ilia Docin
-
Ilia Docin (＠IDocin)