GNOME Bugzilla – Bug 749757
Fix thickness of lines
Last modified: 2015-10-30 23:28:57 UTC
For several versions, the difference between thick and thin grid lines is not nearly big enough. When this was still my codebase, I just went in and changed it on my own version every time I got an update, but now that it's been rewritten, I'll either have to fork to have playable lines or have it fixed. To my eyes, anyway, the thick/thin contrast is *WAY* too low to be playable (in fact, it looks like there may be no thick/thin contrast at all and we're relying only on the difference between grey and black???).
There is actually no thickness difference: all of the lines are the same width, and the only difference is color (gray vs. black). This was intentional. I'm CCing Allan Day, the designer behind this, for his opinion, since I've noticed one or two other complaints about the thin lines. I don't care either way, except that I have a natural instinct to placate both designers and original maintainers, and it seems I can't do both here!
Even if it is quite surprising at first time, I prefer the current look. But I know there’s an issue here, notably for visually impaired people. I’d like to go with a theme chooser, but that’s a really big thing (even if needed), and that’s not my top priority for now.
A quick google images search (https://www.google.com/search?q=sudoku&safe=active&source=lnms&tbm=isch) suggests that nearly all other sudoku have a thick/thin contrast. Even the images that at first glance look most like the current gnome-sudoku look still have a noticeable thick/thin contrast when you zoom in (e.g. http://feelgrafix.com/data_images/out/27/955050-sudoku.jpg). I think we can trust the convention here.
Created attachment 314257 [details] [review] Proposed Changes to the Line Width. Here I have increased the line width from 1 to 1.5 in the case of the main outline and grid boxes.
Comment on attachment 314257 [details] [review] Proposed Changes to the Line Width. Thanks for your contribution. This is a binary file; my ugess is a PNG file created with gnome-screenshot. Please, take a look at https://wiki.gnome.org/Git/Developers#Contributing_patches for instructions on how to submit patches.
Created attachment 314262 [details] [review] Fixing line widths by increasing the line width of the sub grids to 1.5 from 1 https://bugzilla.gnome.org/show_bug.cgi?id=749757
Created attachment 314277 [details] Screenshot
Review of attachment 314262 [details] [review]: It looks blurry to me. :( Probably we need to adjust the position of everything else accordingly, so the stroke doesn't overlap with the cells? You might look at how the original Sudoku code (which used width 2) handled this: https://git.gnome.org/browse/gnome-sudoku/tree/src/lib/sudoku_thumber.py?h=gnome-3-12
Created attachment 314351 [details] [review] New patch boosts up width to 2 to compensate for bluriness,now it looks more clear and cleaner
Created attachment 314352 [details] Picture with line width set to 2.
Comment on attachment 314351 [details] [review] New patch boosts up width to 2 to compensate for bluriness,now it looks more clear and cleaner >From 7df804250f69f4d7e7b9f24059f86dec7c7fd880 Mon Sep 17 00:00:00 2001 >From: Karanbir Chahal <karanbleep@gmail.com> >Date: Thu, 29 Oct 2015 05:28:21 +0530 >Subject: [PATCH] New patch boosts up width to 2 to compensate for > bluriness,now it looks more clear and cleaner > >https://bugzilla.gnome.org/show_bug.cgi?id=749757 >--- > src/sudoku-view.vala | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/src/sudoku-view.vala b/src/sudoku-view.vala >index cbd5b25..b59113c 100644 >--- a/src/sudoku-view.vala >+++ b/src/sudoku-view.vala >@@ -603,7 +603,7 @@ public class SudokuView : Gtk.AspectFrame > c.line_to (board_length, ((int) (i * tile_length)) + 0.5); > } > c.stroke (); >- c.set_line_width (1.5); >+ c.set_line_width (2); > c.set_source_rgb (0.0, 0.0, 0.0); > for (var i = 0; i <= game.board.cols; i += game.board.block_cols) > { >-- >2.1.4
Review of attachment 314351 [details] [review]: OK, the code is fine. Please use 'git commit --amend' to rewrite the commit message according to the guidelines at https://wiki.gnome.org/Git/CommitMessages and then I will push this. Thanks!
Created attachment 314362 [details] [review] Fixing Thickness: Increase width of lines specifying the grid to better help the player to identify what grid he's currently in. Previously the thickness of all the lines of both the number boxes and grids were same , the only thing to differentiate between the two was the colour difference between the lines.This was unsatisfactory hence this commit fixes the problem by increasing the line width of the lines drawn to specify the 9 grids. The line width has been incresed from 1 px to 2 px. https://bugzilla.gnome.org/show_bug.cgi?id=749757 This is only a one line change using the function set_line_width();
Is this fine? Thank you !
Review of attachment 314362 [details] [review]: Better, but one more change please: "First line (the brief description) must only be one sentence and should start with a capital letter unless it starts with a lowercase symbol or identifier. Don't use a trailing period either. It's recommended to not exceed 50 characters but it's not always possible/easy to follow this limit. In any case, don't exceed 72 characters."
Created attachment 314425 [details] [review] Fixing Thickness:Increase width of lines that specify the grid of the sudoku box Previously the thickness of all the lines of both the number boxes and grids were same , the only thing to differentiate between the two was the colour difference between the lines.This was unsatisfactory hence this commit fixes the problem by increasing the line width of the lines drawn to specify the 9 grids. The line width has been increased from 1 px to 2 px. https://bugzilla.gnome.org/show_bug.cgi?id=749757 This is only a one line change using the function set_line_width();
Comment on attachment 314362 [details] [review] Fixing Thickness: Increase width of lines specifying the grid to better help the player to identify what grid he's currently in. >From c1c83a84024412affc3f68adb68c7514675b000a Mon Sep 17 00:00:00 2001 >From: Karanbir Chahal <karanbleep@gmail.com> >Date: Thu, 29 Oct 2015 07:18:56 +0530 >Subject: [PATCH] Fixing Thickness: Increase width of lines specifying the grid > to help the player to identify what grid he's in. > >Previously the thickness of all the lines of both the number boxes and >grids were same , the only thing to differentiate between the two was >the colour difference between the lines.This was unsatisfactory hence >this commit fixes the problem by increasing the line width of the lines >drawn to specify the 9 grids. >The line width has been incresed from 1 px to 2 px. > >https://bugzilla.gnome.org/show_bug.cgi?id=749757 >This is only a one line change using the function set_line_width(); >--- > src/sudoku-view.vala | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/src/sudoku-view.vala b/src/sudoku-view.vala >index 7172b5d..9f9edb4 100644 >--- a/src/sudoku-view.vala >+++ b/src/sudoku-view.vala >@@ -603,7 +603,7 @@ public class SudokuView : Gtk.AspectFrame > c.line_to (board_length, ((int) (i * tile_length)) + 0.5); > } > c.stroke (); >- >+ c.set_line_width (2); > c.set_source_rgb (0.0, 0.0, 0.0); > for (var i = 0; i <= game.board.cols; i += game.board.block_cols) > { >-- >2.1.4
Comment on attachment 314425 [details] [review] Fixing Thickness:Increase width of lines that specify the grid of the sudoku box So, we're actually serious about the 72 character limit for the first line. :) Some distros have a git vim modes that actually highlights your text in red if you go beyond the limit, but I've never figured out how to enable it in other distros.... "view: Increase line width" would be a fine message for the first line.
Created attachment 314521 [details] [review] view: Increase line width Previously the thickness of all the lines of both the number boxes and grids were same , the only thing to differentiate between the two was the colour difference between the lines.This was unsatisfactory hence this commit fixes the problem by increasing the line width of the lines drawn to specify the 9 grids. The line width has been increased from 1 px to 2 px using the function set_line_width.
No problem:) I'm new here and its always nice to adhere with the organisation's standards.
Thanks for your patch :) Attachment 314521 [details] pushed as 1cbb63f - view: Increase line width