lbalazscs / Pixelitor

A desktop image editor
https://pixelitor.sourceforge.io/
GNU General Public License v3.0
182 stars 71 forks source link

Added a little new feature which probably caused a shock to the funda… #320

Closed Minecraftian14 closed 1 year ago

Minecraftian14 commented 1 year ago

Referring to the discussion at https://github.com/lbalazscs/Pixelitor/discussions/318#discussioncomment-7029572 the following changes have taken place:

Minecraftian14 commented 1 year ago

With just a single line change of code, we can allow the randomiser to use the newly added blanks and blacks.

The usual rendom:

The new random:

Both have their own charms!

Minecraftian14 commented 1 year ago

One more issue, did you notice the 'thin' and 'thick' lines in diagonals pattern? How to fix that?

lbalazscs commented 1 year ago

A problem is that the new pattern can't be selected from the combobox. For this, we also need a new name. I'll leave it to you to choose — it can be a gemstone or "Houndstooth." This seems to be the standard name for this pattern, it even has a Wikipedia page at https://en.wikipedia.org/wiki/Houndstooth

Note that with this "extended Truchet", new patterns will be possible. For example, I like

    private final int[][] ARRAY_23 = {
        {4, 3},
        {1, 5},
    };

So we shouldn't assume that in the future, all the patterns will fit into the A-Z range, each corresponding to only one letter, because we might run out of letters.

Perhaps there could be a "Random 2" or "Extended Random" (or invent a more creative name). I prefer the old random (it should not be removed), but the new random is also interesting sometimes.

I think the 'thin' and 'thick' lines in DIAGONALS are caused by the fact that you draw at the border of the tile. No matter how thick or thin you draw it, the overall thickness will also depend on the unpredictable neighboring tile. The rule in the Truchet game is that each tile must be drawn in a way that looks acceptable with any neighbors. Can you achieve a nice effect with the new tiles without drawing at the border? Perhaps by not drawing anything, or maybe createBlankArea could simply call createFilledArea?

Minecraftian14 commented 1 year ago

Missing entry in drop menu

It's strange that the line vanished during the commit 🤔 I had added that line to add wavellite into the menu, but it got removed. I probably messed up the rebase...

New patterns with blancks and blacks

Regarding new patterns possible, maybe we can come up with a few designs. A truchet overhaul 😂

Naming convention?

We can simply start over from A when we reach Z. Check out Ubuntu version names! (Navigate to Table of versions) https://en.m.wikipedia.org/wiki/Ubuntu_version_history

The new random filter

My problem was with how a lot of final constants were added for each pattern. Clearly, you have tried to put patterns with a smaller repeating tile before the larger ones. I was already uncomfortable adding Wavellite at 22, I wondered if Random 2 should be at 23 or 1 pushing everything below?

Think and thins

Lemme see if I can make a suitable change.

lbalazscs commented 1 year ago

We can simply start over from A when we reach Z. Check out Ubuntu version names!

In order to be really Ubuntu-like, we should add alliterating adjectives, like "Alluring Aquamarine", "Bumbling Baryte", "Cheeky Citrine", "Dazzling Diamond", "Enchanting Emerald", "Frisky Friedelite", "Gleeful Garnet", "Hilarious Hambergite", "Inventive Iolite", "Jolly Jade", "Kind-hearted Kyanite", "Laughing Lapis", "Merry Moonstone", "Nimble Neptunite", "Optimistic Opal", "Playful Pearl", "Quirky Quartz", "Radiant Ruby", "Silly Sapphire", "Teasing Turquoise", "Unstoppable Uvite".

These were generated by ChatGPT, and are only a joke from me, I don't actually want to add adjectives. Seriously, we have two issues: (1) we're running out of letters (2) there's no clear distinction between the real Truchet patterns and our "extended Truchet". I think we could solve both problems by restarting the alphabet right now, but this time using a different naming scheme, such as chemical element names (which was your other initial idea)

I was already uncomfortable adding Wavellite at 22, I wondered if Random 2 should be at 23 or 1 pushing everything below?

I don't know, neither seems ideal. Perhaps it would be best to forget the new random feature if it causes more confusion than it adds value...

lbalazscs commented 1 year ago

I love the new patterns, but I'm less enthusiastic about the "random tiles."

The trick to fix the diagonal patterns is quite clever, it even fixes an old problem with the diagonal patterns (the lines used to appear dashed at the tile boundaries).

However, there seems to be a bug when the tile size is an odd number: the tile boundaries become visible (if the triangles type is selected), and the line width also changes (if the diagonal type is used).

Minecraftian14 commented 1 year ago

Sorry for late commits, feel free to continue your works on Pixelitor. Class schedules in my college are all upside down, gobbling our energies leaving nothing for us.

I believed that the random tiles was a way to explore some patterns which we should have added in the preset list. While also giving the user to explore some funky and some decent patterns without any origin, free for their own nomenclature :laughing:... Anyway, not a big deal - I removed that feature.

As for the diagonol pattern fix, thanks for the compliment :sweat_smile:. I had a rough idea that I might face it - and the solution was just to add a simple padding around so that drawing can continue outside the tile - while moving the background drawing outside the tile drawing so that it can be transparent (which further helps preventing tiles from overdrawing background on others foreground).

About the odd tile count bug... It's related to anti aliasing... It can also be seen in some even tile count cases where the tile size is odd. I believe that math is correct (too bored to state a proof though). So the issue can be dodged by forcing the coordinates to stick to integral values instead of double. I sprinkled a bunch of floor and ceil wrapping around the coordinates in the code - which further contributes to the code complexity. So I'll just add a few notes no one wants, to reason why what where is after the comment. Please ignore that comment if you don't link it.

As a final sentence for this comment, good bye random tiles :sneezing_face: image

Minecraftian14 commented 1 year ago
lbalazscs commented 1 year ago

Don't worry about holding me up with a pending PR, because if I want to make a change, I can always do it on a branch and rebase when the PR is merged.

Regarding "random tiles": I like the idea and the way they look, but it seemed to me that they make the user interface confusing. Even the name is confusing: "tile" in the context of Truchet tiling refers to the individual rotated images, but here it's used to describe the repeating pattern of tiles. However, we can't simply call it a "random pattern" since there's already a pattern called "random"...

The distinction between "random" and "ex random" might already be confusing for someone unfamiliar with the implementation details or how we decided to extend Truchet. Just imagine that you see it for the first time... A smaller issue is that the user has no control or feedback over the size of the selected pattern (this could be improved by simply adding a "show pattern boundary" checkbox in addition to the "show tile boundary" checkbox).

So the idea of "random tiles" can be reintroduced if we can make the user interface less confusing. Alternatively I can merge the PR as is, since it's already a great improvement.

lbalazscs commented 1 year ago

I decided that the silence means I should merge the PR as it is, so I'm doing that. (Feel free to start a new discussion or PR if I was wrong.)