I accidentally skipped this part of the spec in the QOI decoder:
> The alpha value remains unchanged from the previous pixel.
This led to incorrect rendering of some images with transparency,
visible in form of a horizontal line of non-transparent pixels (that
shouldn't exist), e.g. for the following chunk sequence:
- QOI_OP_RGBA with alpha = 0
- QOI_OP_RGB
- QOI_OP_RUN
The QOI_OP_RGB should 'inherit' the alpha value of the previous
QOI_OP_RGBA chunk, instead of always setting it to 255.
I'm unsure why the encoder added the QOI_OP_RGB chunk to the specific
image where the bug was noticed in the first place - they effectively
both had fully transparent color values.
This wasn't quite following what the spec says for step 2:
"If index is −1, then remove the last element in the rows collection
from its parent, or do nothing if the rows collection is empty."
It was behaving like:
"If index is −1 and the rows collection is not empty, then remove the
last element in the rows collection from its parent."
Which is not the same, as it will fall into the "Otherwise" if
`index == -1` and the rows collection is empty and try and get the -2nd
element of the rows.
Found with Domato.
Step 1 of the spec is to capture the <script> element's node document
into a local variable.
When I originally implemented this, I thought this was not necessary.
However, I realised that the script that runs can adopt the current
script element into a different document, meaning step 5.4 and 6 then
operate on the incorrect document.
Covered by this WPT: 7b0ebaccc6/html/semantics/scripting-1/the-script-element/moving-between-documents-during-evaluation.html
Previously, we would use lossy strtod() conversion. This was bad,
especially since we switched from internally storing Calculator
state in a double to storing it in the KeypadValue class
some time ago. This commit adds a constructor for the KeypadValue
class that is not lossy by using strtoll(). It handles numbers
with and without decimal points as well as negative numbers
correctly.
In order to reduce our reliance on __builtin_{ffs, clz, ctz, popcount},
this commit removes all calls to these functions and replaces them with
the equivalent functions in AK/BuiltinWrappers.h.
...by flattening the underlying bytecode chunks first.
Also avoid calling DisjointChunks::size() inside a loop.
This is a very significant improvement in performance, making the
compilation of a large regex with lots of alternatives take only ~100ms
instead of many minutes (I ran out of patience waiting for it) :^)
The generated data for libunicodedata.so is quite large, and loading it
is a price paid by nearly every application by way of depending on
LibRegex. In order to defer this cost until an application actually uses
one of the surrounding APIs, dynamically load the generated symbols.
To be able to load the symbols dynamically, the generated methods must
have demangled names. Typically, this is accomplished with `extern "C"`
blocks. The clang toolchain complains about this here because the types
returned from the generators are strictly C++ types. So to demangle the
names, we use the asm() compiler directive to manually define a symbol
name; the caveat is that we *must* be sure the symbols are unique. As an
extra precaution, we prefix each symbol name with "unicode_". For more
details, see: https://gcc.gnu.org/onlinedocs/gcc/Asm-Labels.html
This symbol loader used in this implementation provides the additional
benefit of removing many [[maybe_unused]] attributes from the LibUnicode
methods. Internally, if ENABLE_UNICODE_DATABASE_DOWNLOAD is OFF, the
loader is able to stub out the function pointers it returns.
Note that as of this commit, LibUnicode is still directly linked against
LibUnicodeData. This commit is just a first step towards removing that.
Loading libunicodedata.so will require dlopen(), which in turn requires
mmap(). The 'prot_exec' pledge is needed for this.
Further, the .so itself must be unveiled for reading. The "real" path is
unveiled (libunicodedata.so.serenity) as the symlink (libunicodedata.so)
itself cannot be unveiled.
This is a deprecated feature that is still in use in some older games,
most notably Grim Fandango in ScummVM makes heavy use of it.
Luckily, since you can only draw convex polygons, the end result is
exactly the same as when you would have used `glBegin(GL_TRIANGLE_FAN)`
- so we just reuse that code as-is.
Implement support for the `GL_TEXTURE` matrix mode, the texture matrix
stack and texture coordinate matrix transformation.
Also, an unused `m_current_matrix` was removed to make room for
`m_texture_matrix`.
In OpenGL, texture coordinates can have up to 4 values. This change
will help with easy application of texture coordinate matrix
transformations in the future.
Additionally, correct the initial value for texture coordinates to
`{ 0.f, 0.f, 0.f, 1.f}`.
The actual value is unchanged, but the previous `0xffffffff` was an
unsigned value, which lead to clang getting mad at `foowc() == WEOF`.
This commit makes it a signed int on clang, which *should* serve
the same purpose and not lead to clang getting mad at us.
What the component which did the actual decoding is called is not
relevant for the error, and would be rather distracting once we show
decoding error messages e.g. in ImageViewer (instead of just silently
failing).
Also makes them more consistent as many already don't include it - a
mistake which is now turned into a feature :^)
This also replaces an instance of TRY with MUST as the spec indicates
that step 35b of RegExpBuiltinExec cannot throw. Further, this moves
some lines of code around to align with the spec as best as we can,
though the end effect is the same.
In doing so, this caught an erroneous ToObject invocation. In the one
spot that requires the value to be an object (the invocation to
LengthOfArrayLike), we know by then the value is an object because all
other possibilities have been handled.
In doing so, this fixes a few minor issues:
1. We were accessing the "unicode" and "lastIndex" properties out of
order. This is somewhat frequently tested by test262, but not in
this case.
2. We were doing a Value::to_object() followed by Object::get(), rather
than just Value::get() as the spec dictates.
3. We were TRYing a step (CreateDataPropertyOrThrow) that should never
fail, and should have been a MUST.
RegExpExec already returns a ThrowCompletionOr so this potentional error
should be a completion. RegExp.prototype [ @@replace ] is tripped up by
this mistake when implemented closer to the spec.
Before this a closing html comment would not be treated as a comment if
directly following a block comment which was not the first token of its
first line.
We can guess it both from the magic bytes 'qoif' or the file extension
'.qoi'. The mime type is made up, I don't think it has an official one
yet - using the 'x-' prefix should be fine though.
The spec had its first stable release today, so I figured we should
support it as well!
As usual, by using the regular LibGfx image decoder plugin architecture,
we immediately get support for it everywhere: ImageViewer, FileManager
thumbnails, PixelPaint, and (with a small change in the subsequent
commit) even the Browser :^)
Some icons only exist in 16x16 and we should still allow them to be
loaded via the LibGUI default icon system.
This patch also reimplements GUI::default_icon() as a MUST() wrapper
around try_create_default_icon().
This fixes the problem before, where searching "Shell" would list
"Shell-vars" in the results, but searching "Shell-vars" would make it
disappear.
Also removed some now-unnecessary includes.
I found it strange that `man` and `Help` did not accept the same command
line arguments since they are so similar. So... now they do. :^)
This means you can now open for example the `tar` man page in Help with
`Help tar`, or `Help 1 tar` if you want to disambiguate between pages in
different sections.
If the result is not found, it falls back to the previous behavior,
treating the input as a search query.
Initially I had this written as two optional positional arguments, but
when told to parse `[optional int] [optional string]`, and then given a
string input, ArgsParser forwards it to the [optional int], which then
fails to parse. Ideally it would pass it to the second, [optional
string] arg instead, but that looks like a fairly big change to make to
ArgsParser's internals, and risk breaking things. Maybe this ugly hack
will be an incentive to fix it. :^)
Previously, launching Help with a query like `Help tar` left the page
blank, which looks like something has gone wrong. Instead, let's show
the usual welcome page.
This patch adds a 512 frame timeline to Magnifier and the ability to
step through it with the arrow keys.
This makes it easier to check Serenity animations frame by frame for
correctness etc.
Before, `SoftwareRasterizer` was iterating over all 32 possible texture
units for each fragment and checking each if they're bound to a texture.
After this change, an intrusive list containing only texture units with
bound textures is passed to the rasterizer. In GLQuake, this results in
a performance improvement of ~30% (from 12 to 16 FPS in the first demo)
on my machine.
This smaller block size allows us to use an `u8` for the pixel mask
during triangle rasterization. These changes result in better FPS in
3DFileViewer, which periodically shoots up to 250-300 FPS on my
machine. Before, the peaks were somewhere in the range of 160-170 FPS.
Previously we multiplied the interpolated texture coordinates by
width - 1 and height - 1 instead of width and height which resulted in
some wrongly mapped textures, especially visible in the glquake light
maps.
This also corrects the wrap mode being wrongly swapped for s/t
coordinates.
Since we do not have texture borders implemented yet we always use
GL_CLAMP_TO_EDGE for all clamping wrap modes for the time being.
Reduced focus rect inflation value for buttons with icons
to match the expected focus rect for buttons without icons.
As mentioned in 'SerenityOS Office Hours / Q&A (2021-12-17)'
Not much of a difference as TimeFraction just parses Fraction, but let's
do it correctly. Small mistake I did in 4b7f716.
Thanks to YouTube user gla3dr for noticing this :^)
The commandline "notify" application was always attempting to load an
icon path from an optional argument, even when the argument was
omitted. In this case, the image icon argument would be a null pointer
and the notify program would crash.
This fix adds a conditional to only attempt to load the icon file if
the icon_path variable is not a null pointer
Both `AK/Assertions.h` and `assert.h` would define the macro if `NDEBUG`
is set.
Remove the definition from `assert.h` since it is not an ISO-C
requirement.
