This refactor includes:
- In Lua now we can clone a custom brush with Brush(Image) and the new
brush doesn't share the image with the original one (added a new test
for this).
- Avoid creating extra images when it's not needed using
Brush::cloneWithExistingImages() (we can inject existing images in
the brush itself).
- Delete Brush-copy contructor & assign operator to use
Brush::clone() functions instead (which are more explicit).
- Some code from 12d8135264 (#4023)
reverted to avoid recreating brushes on left-click or in the brush
preview, i.e. moving the mouse (#4013 refers only to right-click, so
only on right-click we have to adjust the custom brush).
Before this fix to change the main color of the image brush,
it was necessary to choose a new color from the palette.
The secondary color can now also be used.
Also added some tests for image brushes.
This errors was reported in #4431: The Tx wasn't rolled back correctly
in case of a Lua error inside the transaction because Lua needs to be
compiled as C++ to avoid longjmps and support stack
unwinding (i.e. calling destructors).
If the user preferences file (aseprite.ini) contained a non-existent
language, the first option of the languages combo box was selected,
which might lead to a confusing situation where just opening the
preferences dialog will change from English to other
language (non-English, the first language in the combobox).
This was mainly found in SpriteSizeJob crash reports. In these reports
deleted image buffers were still used to paint the Editor canvas
because the doc was write-locked in the main thread (same thread where
the canvas is painted). This produced a re-entrant lock in the
Editor::onPaint() as we can still read-lock from the same thread where
we write-locked the doc.
With this change we write-lock the doc from the SpriteJob background
thread (not the main thread) only if it's necessary (i.e. when the doc
is not already locked in the main thread, e.g. when running a script).
This makes that the main thread (Editor::onPaint) cannot read the doc
until we finish the whole SpriteJob transaction/Tx.
One fix that this patch includes for #4367 (crash by unhandled
CannotWriteDocException exception) is when we drag-and-drop a tag
border to resize its limits. This is a fix for that case but we don't
know if this include all cases of #4367 crashes (so we cannot close
the issue).
Anyway we have added some try { } catch in cases where it's better to
avoid propagating the exception, e.g. in
MovingSelectionState::onLeaveState() to avoid throwing exceptions in
Editor::backToPreviousState() which might be problematic in several
cases. (Maybe related to #2829?)
Anyway this code is not used anymore, it was for v1.1 when v1.1 and
v1.2 branches were developed at the same time (layer groups feature
was added in v1.2, so v1.1 just moved groups children to the root).
This fixes several problems in MovingPixelsState where hidden layers
were transformed anyway when we switched the visibility of a layer in
this state.
Other fix was tried before in #3254 but we needed the onBefore/After
layer visibility change notifications to make this work properly
(i.e. drop pixels when the visiblity of a layer is changed).
The only drawback at this moment is that changing the visibility of
the non-active layer when we are transforming multiple cels/timeline
range can be confused because we don't have #2144/#2865 implemented
yet.
This bug was originally reported here: https://community.aseprite.org/t/20621
The warning is about using a virtual member function in the destructor
where it can be overridden by a derived class so in this case the
derived version wouldn't be called.
Just testing clang-tidy to see if we can add something in the CI.
Before this fix, an incomplete custom theme or an outdated official
theme could cause a crash during Aseprite startup.
This fix does not alert the artist the problem of the theme.
Simply avoid the crash.
We were incorrectly saving a wrong number of entries for palettes with
more than 256 colors in color2 chunk, anyway it doesn't make sense to
use this chunks as it doesn't support more than 256 colors. So we
removed it for this case.
We've also removed the palette chunk for cases where it's not required
at all, e.g. when we have less than 256 colors and doesn't have alpha
channel, it makes sense to use the color2 chunk as it's smaller in the
output file.
Some minor changes in this commit includes the usage of
std::unique_ptr for the Pimpl-idiom in steam::SteamAPI class and
renaming the SteamAPI::initialized() to SteamAPI::isInitialized() to
avoid confusion reading the code.
Forum post:
https://steamcommunity.com/app/431730/discussions/2/7260435303111061192/
Prior to this fix, Aseprite was hiding some sessions to recover,
resulting in additional support for users.
To display available recovery sessions, Aseprite only checked if
the session belonged to any running process on the OS via the
'pid' number.
Steam API/DLL now contains a SteamAPI_InitSafe() function for backward
compatibility similar to the old SteamAPI_Init(). The official
"steam_api.h" header file contains inline-defined functions for
SteamAPI_Init() and SteamAPI_InitEx() but both use the published
SteamInternal_SteamAPI_Init() function (we don't need it, I guess, so
SteamAPI_InitSafe() is enough for us).
This fixes the ASSERT((mnemonic & kMnemonicModifiersMask) == 0) in
Widget::setMnemonic() function, but as a future task we should try to
process/convert unicode chars like 'Ú' as a simplified form,
i.e. pressing 'u' should match 'ú' for mnemonics.
As we've moved all the translations to the strings
repo (06a852d1d4), to facilitate the
i18n work now we load translations from "data/strings" and from
"data/strings.git" folders, where "strings.git" is a clone of the
strings repo (https://github.com/aseprite/strings.git) in
"build/bin/data/strings.git".
This clone is executed automatically in the cmake configuration stage
so it's transparent for the developer and the result is like having
the translations available in the same "aseprite" repo.
Without this when the playback cue is on the highest frame in "PlayAll"
mode it doesn't take into account the direction of the current tag that
is being played and assumes the end of the animation was reached
This is due zlib returning Z_OK (instead of Z_STREAM_END) after
inflate() when all the output buffer was filled (avail_out = 0) but it
reports like there is still available uncompressed data (avail_in > 0).
It makes no sense but an extra inflate() call with avail_out=0
consumes the whole avail_in and the expected Z_STREAM_END is finally
reported.
Without this an exception in DelayedMouseMove::commitMouseMove() could
produce (e.g.) the crash in #3818. This same error handling was
already done for Editor::onProcessMessage() in
DocView::onProcessMessage() to avoid crashing due unhandled exceptions
in Editor message processing.
Regression introduced with 8722c8ec16
and d3562b140c.
Our re-entrant RWLock implementation is tricky because it doesn't
support to unlock in different order than it was locked.
E.g. If we create a DocWriter (e.g. paste command), and try to create
a DocReader (e.g. PixelsMovement) inside the DocWriter scope, the
reader will return a LockResult::Reentrant, but if we unlock the
write, then the reader cannot be used to upgradeToWrite() because the
lock will have just 0 locks (the re-entrant one didn't modify the
count of the RWLock). This is because it was thought that the
DocReader was going to be unlocked before the writer lock (in the
exact inverse order).
Basically these re-entrant RWLocks will not work fine if we mix up the
order of locks/unlocks in the same thread.
With re-entrant RWLocks we can try to lock the document on each
transaction/command/modification. This fixes several problems running
scripts that weren't locking the sprite in an app.transaction() call.
This already fixes a lot of possible problems that can happen when a
script is running and modifying some part of a sprite that is being
backed up in a background thread.
We still need some work to being able to lock a sprite two or more
times in the same thread to write it. E.g. an app.transaction() should
lock the sprite for write access, but the script transaction function
could call a command, and that command could use a ContextWriter to
lock the sprite again. At the moment this is not possible because we
need a re-entrant RWLock implementation.
