Skip to content

Commit

Permalink
derive default line width from display dpi
Browse files Browse the repository at this point in the history 
allows better default on high dpi displays where the previous
default of 1 pixel would've been too small.

Closes: #285
  • Loading branch information
@N-R-K
N-R-K committed Jan 30, 2025
1 parent 53b5edf commit bd0ffe6
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 3 deletions.
2 changes: 1 addition & 1 deletion configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ AS_IF([test "x$orig_CFLAGS" = "x"], [
AX_APPEND_LINK_FLAGS(["-flto"])
])
m4_foreach([SCROT_FLAG],
[["-O3"], ["-Wall"], ["-Wextra"], ["-Wpedantic"]], [
[["-O3"], ["-Wall"], ["-Wextra"], ["-Wpedantic"], ["-fno-math-errno"]], [
AX_APPEND_COMPILE_FLAGS(["SCROT_FLAG"])
AS_IF([test "x$LTO_ENABLED" = "xyes"], [
AX_APPEND_LINK_FLAGS(["SCROT_FLAG"])
Expand Down
1 change: 1 addition & 0 deletions deps.pc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ Name: scrot's mandatory dependencies
Description: ditto
Version: infinite
Cflags: -D_XOPEN_SOURCE=700L
Libs: -lm
Requires: x11 imlib2 >= 1.11.0 xcomposite >= 0.2.0 xext xfixes >= 5.0.1 xinerama >= 1.1.3
2 changes: 1 addition & 1 deletion man/scrot.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ SELECTION STYLE

Without the -l option, a default style is used:

mode=auto,style=solid,width=1,opacity=100
mode=auto,style=solid,width=$dpi/75,opacity=100

Example:

Expand Down
1 change: 0 additions & 1 deletion src/options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ struct ScrotOptions opt = {
.quality = 75,
.compression = 7,
.lineStyle = LineSolid,
.lineWidth = 1,
.lineOpacity = SELECTION_OPACITY_DEFAULT,
.stackDirection = HORIZONTAL,
.outputFile = defaultOutputFile,
Expand Down
9 changes: 9 additions & 0 deletions src/scrot_selection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

#include <err.h>
#include <errno.h>
#include <math.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -423,6 +424,14 @@ Imlib_Image scrotSelectionSelectMode(void)
opt.lineMode = LINE_MODE_CLASSIC;
}

if (opt.lineWidth == 0) {
double mmToInches = 1 / 25.4;
double inchesDiag = hypot(scr->mwidth, scr->mheight) * mmToInches;
double pixelsDiag = hypot(scr->width, scr->height);
double dpi = round(pixelsDiag / inchesDiag);
opt.lineWidth = fmin(fmax(round(dpi / 75.0), 1), 8);
}

if (opt.delaySelection)
scrotDoDelay();

Expand Down

2 comments on commit bd0ffe6

@guijan
Copy link
Contributor

@guijan guijan commented on bd0ffe6 Feb 1, 2025

Choose a reason for hiding this comment

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

The accuracy of the math really isn't important here, it only has to scale with DPI—floats aren't needed. X gives us millimeters, we can just operate on millimiters instead of converting to inches, too. And the exact logic behind the default choice doesn't have to be documented.

Putting it all together, the code becomes as simple as this:

    if (opt.lineWidth == 0) {
        opt.lineWidth =
            (scr->height + scr->width) / (scr->mheight + scr->mwidth);
        if (opt.lineWidth < 1)
            opt.lineWidth = 1;
        else if (opt.lineWidth > 8)
            opt.lineWidth = 8;
    }

This gives a 1mm wide line, I actually measured it to be sure.

@N-R-K
Copy link
Collaborator Author

@N-R-K N-R-K commented on bd0ffe6 Feb 6, 2025

Choose a reason for hiding this comment

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

The accuracy of the math really isn't important here

Yes, I was planning to get rid of it too.

This gives a 1mm wide line

This makes the default line 4px from the previous 1px default on my screen. Which seems a bit excessive. I've divided the value for 4 in 2ea7a12.

And the exact logic behind the default choice doesn't have to be documented.

We should have some documentation about the fact that the default line width isn't static and depends on resolution + screen size.

Please sign in to comment.