EXECUTE_PROCESS() has been around since at least 3.0, and EXEC_PROGRAM()
is deprecated as of 3.28:
CMake Warning (dev) at cmake/cmake_uninstall.cmake:12 (EXEC_PROGRAM):
Policy CMP0153 is not set: The exec_program command should not be called.
Run "cmake --help-policy CMP0153" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Use execute_process() instead.
On Qt4 and Qt5, we could add a stretch layout item to the
QDialogButtonBox's layout directly. On Qt6, this doesn't seem
to work properly, since a bunch of internals were changed.
Using HelpRole instead of ActionRole seems to provide the same effect
on Qt6, and I tested that it works on Qt5 as well, so it will probably
work on Qt4. (TODO: Test it.)
Note that since the button has HelpRole, it might cause the
QDialogButotnBox to emit the helpRequested() signal. Nothing appears
to connect to this signal by default, so it's not an issue.
There's a bit of weirdness with Intellivision due to its unusual word
size. The General Instrument CP1610 is a 16-bit CPU, but uses 10-bit
opcodes. Because of this, Mattel used 10-bit ROMs. Some of the fields
in the ROM header use 16-bit addresses, split up into two 10-bit words.
The ROM file uses 16-bit big-endian for words, so in order to decode
these addresses, we have to take two 16-bit big-endian values, read
the low 8 bits of each, and combine it into a 16-bit value.
NOTE: ROM images must have a .int or .itv file extension, since the
Intellivision ROM header doesn't have a magic number.
The base classes are never used directly, so we don't need MSVC to emit
vtables for them.
Code size difference: (64-bit Windows 7, MSVC 2022 17.6.5, release build, with LTO)
text data bss dec hex filename
1606944 14976 0 1621920 18bfa0 romdata-4.dll [before]
1606628 14976 0 1621604 18be64 romdata-4.dll [after]
-316 0 0 -316 -13c Difference
text data bss dec hex filename
532639 7776 0 540415 83eff rom-properties.dll [before]
529255 7776 0 537031 831c7 rom-properties.dll [after]
-3384 0 0 -3384 -d38 Difference
Make it a non-virtual function that returns a new member variable
m_hasDefaults. This variable is initialized in the ITab constructor
using an optional parameter, which defaults to true.
This does increase the class size slightly (by 1 or 4 bytes), but
the overall code size is decreased due to the removal of a virtual
function.
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, no LTO)
text data bss dec hex filename
584571 22016 80 606667 941cb rom-properties-kf5.so [before]
584499 22016 80 606595 94183 rom-properties-kf5.so [after]
-72 0 0 -72 -48 Difference
text data bss dec hex filename
597810 29384 80 627274 9924a rom-properties-kf6.so [before]
597758 29384 80 627222 99216 rom-properties-kf6.so [after]
-52 0 0 -52 -34 Difference
NOTE: Can't do this for the GTK version because glib interfaces don't
allow storing any data. (C++ doesn't really have "interfaces"...)
The Win32 version uses messages to enable/disable the "Defaults"
button instead of a variable or virtual function.
QSize contains two 32-bit int values, which can be passed in a single
64-bit register (or two 32-bit registers). Doing so reduces memory
accesses and pointer indirections.
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, no LTO)
text data bss dec hex filename
584587 22016 80 606683 941db rom-properties-kf5.so [before]
584571 22016 80 606667 941cb rom-properties-kf5.so [after]
-16 0 0 -16 -10 Difference
text data bss dec hex filename
597810 29384 80 627274 9924a rom-properties-kf6.so
597810 29384 80 627274 9924a rom-properties-kf6.so
0 0 0 0
TODO: Find more small Qt structs that are currently being passed by
const ref and change them to be passed by value.
AExp uses the alpha channel as a scaling value for RGB.
The alpha channel is set to 255 after decoding, so the image
effectively has no alpha channel. (fully opaque)
We don't need to check errno for any math functions, and this prevents
certain inlining optimizations. In particular, lrintf() can't be inlined
if we don't specify this, even though glibc's documentation says that
its lrintf() implementation doesn't set errno.
Assembly comparison of manual truncation vs. lrintf() with
-fno-math-errno:
amd64:
- Manual: cvttss2si
- lrintf(): cvtss2si (note the single t)
i386 with SSE disabled:
- Manual: fistp with control word manipulation
- lrintf(): fistp without control word manipulation
Presumably the control word on manipulation changes the rounding mode.
lrintf() without errno doesn't change the rounding mode and relies on
the default rounding, which matches amd64.
TODO: MSVC equivalent, if available.
Casting to uint8_t has different behavior on 64-bit amd64 compared to
32-bit i386, since i386 defaults to x87 math.
NOTE: Specifying -mfpmath=sse on i386 results in the same behavior as
amd64, but we can't rely on that.
On amd64: (Gentoo Linux)
- 0.235294 * 255.0f -> 60.000000
On i386: (Ubuntu 16.04)
- 0.235294 * 255.0f -> 59.999996
This broke the YCoCg image tests. With lrintf() and re-generated YCoCg
images, it now passes on both amd64 and i386.
NOTE: Simply using lrintf() without re-generating the YCoCg images
doesn't work, since the truncation results didn't always match how
lrintf() handles values. However, lrintf() is consistent across
CPU architectures, so this fixes things everywhere.
NOTE: AppVeyor uses Ubuntu 18.04, which might explain why it isn't
failing there...
It seems to be on by default for KF5 and KF6, but let's explicitly
turn it on anyway.
[kde4] Removed CMAKE_AUTOMOC=ON, since it's now in the base kde directory.
The subdirectories are needed in order to prevent collisions with e.g.
different versions of automoc.
When building for KDE4 and KF5 on Ubuntu 16.04, with tests enabled,
this happened:
[ 81%] Building CXX object src/kde/tests/CMakeFiles/ListDataSortProxyModelTest_kf5.dir/ListDataSortProxyModelTest_kf5_automoc.cpp.o
In file included from src/kde/tests/ListDataSortProxyModelTest_kf5_automoc.cpp:2:0:
src/kde/tests/moc_ListDataModel.cpp:13:2: error: #error "This file was generated using the moc from 4.8.7. It"
.#error "This file was generated using the moc from 4.8.7. It"
^
src/kde/tests/moc_ListDataModel.cpp:14:2: error: #error "cannot be used with the include files from this version of Qt."
.#error "cannot be used with the include files from this version of Qt."
^
src/kde/tests/moc_ListDataModel.cpp:15:2: error: #error "(The moc has changed too much.)"
.#error "(The moc has changed too much.)"
^
Using subdirectories fixes this.
kde/tests/kde4: Set CMAKE_AUTOMOC=ON, since it defaults to OFF for
some reason. (TODO: Do this in the base kde directory?)
This file is included by svrplus.c, so we can't use C++ namespace headers.
Otherwise, we end up with a wacky error:
MSVC\14.36.32532\include\yvals_core.h(28): STL1003: Unexpected compiler, expected C++ compiler.
MSVC\14.36.32532\include\yvals_core.h(29,1): fatal error C1189: #error: Error in C++ Standard Library usage
- KDE4: Remove the `QT_VERSION < 0x050000` check. This is a remnant
from a while back when I attempted to use the same file for both
versions, which didn't work.
See commit 4c5122d7de.
(Added preliminary KDE5 support.)
- struct _RpThumbnailer: Rearrange fields to reduce the struct size.
On 64-bit, reduced from 104 bytes to 96 bytes.
- Use G_DEFINE_TYPE_EXTENDED() instead of manually defining types. There
was a comment saying we couldn't do this because G_DEFINE_DYNAMIC_TYPE()
requires a GTypeModule, but we don't need to use the dynamic version.
- The class_init and init functions lose their data parameters when
using G_DEFINE_TYPE_EXTENDED(). We weren't using those parameters
anyway, so that's not a problem.
- rp_thumbnailer_process(): Use g_strdup_printf() instead of the
complex and potentially error-prone manual calculation. It might use
some more memory and CPU cycles due to two strings (dir and filename),
but is likely to be more stable (and is definitely more readable).
[libromdata] Nintendo3DSFirm: Fix a strict aliasing error on Ubuntu 16.04. (gcc-5.4.0)
In file included from src/libromdata/stdafx.h:62:0,
from src/libromdata/Handheld/Nintendo3DSFirm.cpp:9:
src/libromdata/Handheld/Nintendo3DSFirm.cpp: In member function ‘virtual int LibRomData::Nintendo3DSFirm::loadFieldData()’:
src/libromdata/Handheld/Nintendo3DSFirm.cpp:369:97: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
const uint32_t first4 = be32_to_cpu(*(reinterpret_cast<const uint32_t*>(firmHeader->signature)));
^
src/libromdata/../librpcpu/byteswap_rp.h:63:52: note: in definition of macro ‘__swab32’
# define __swab32(x) ((uint32_t)__builtin_bswap32(x))
^
src/libromdata/Handheld/Nintendo3DSFirm.cpp:369:27: note: in expansion of macro ‘be32_to_cpu’
const uint32_t first4 = be32_to_cpu(*(reinterpret_cast<const uint32_t*>(firmHeader->signature)));
^
cc1plus: some warnings being treated as errors
Otherwise, we can end up with a valid file (isOpen() returns true),
but a NULL png_ptr, resulting in assertions or bogus errors.
This happened when $HOME wasn't writable due to the bad AppArmor profile.
It's not allowing writes to the user's home directory, which prevents
extracting images.
TODO: Get it working...
The rp-download profile is still enabled, since it's only allowed to
write to the cache directory.
The path was changed to a common KIO directory in 5.85.
rom-properties-kf5.install: Add the ${SERVICES_INSTALL_DIR} entry, and
also remove the KIO install entry, since that was split out into the
rom-properties-kio-servicemenus package.
TODO: Don't install rom-properties-kio-servicemenus if KF5 is earlier
than 5.85, or install it anyway for forward compatibility?
[kf5] On Ubuntu 16.04, ${SERVICES_INSTALL_DIR} is incorrectly set to
the KDE4 directory. Not usre if it's because kdelibs5 for KDE4 is also
installed... As a workaround, if ${SERVICES_INSTALL_DIR} contains kde4,
change it to "share/kservices5".
ServiceTypes is likely only used by KF5. (Unsure when it was switched
from X-KDE-ServiceTypes to ServiceTypes.)
KF6 does not use either ServiceTypes key.
They were deprecated in KF5 5.89, but it turns out they weren't actually
needed in KDE4 either:
- Keyword is defaulted to QString() in older versions. In newer
versions, there's an overload that doesn't take a keyword.
- Instantiation is done based on inheritance.
Tested on Ubuntu 16.04 with dolphin-15.12.3 and KF5 5.18.0.
TODO: Verify that this works on KDE4.
Same changes that were done for ExtractorPlugin.
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, no LTO)
text data bss dec hex filename
588702 22272 80 611054 952ee rom-properties-kf5.so [before]
584587 22016 80 606683 941db rom-properties-kf5.so [after]
-4115 -256 0 -4371 -1113 Difference
text data bss dec hex filename
17374 1088 48 18510 484e overlayiconplugin_rom-properties-kf5.so [before]
26116 1424 48 27588 6bc4 overlayiconplugin_rom-properties-kf5.so [after]
+8742 +336 0 -9078 -2376 Difference
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, with LTO)
text data bss dec hex filename
535457 22224 72 557753 882b9 rom-properties-kf5.so [before]
532309 22000 72 554381 8758d rom-properties-kf5.so [after]
-3148 -224 0 -3372 -d2c Difference
text data bss dec hex filename
17314 1088 40 18442 480a overlayiconplugin_rom-properties-kf5.so [before]
25327 1424 40 26791 68a7 overlayiconplugin_rom-properties-kf5.so [after]
+8013 -336 0 -8349 -209d Difference
The size increase in overlayiconplugin is more than double the decrease
in the main plugin this time, even with LTO...
Metadata extraction was originally added in v1.4, but the shared
libromdata.so wasn't implemented until v2.0. The forwarder plugin
made sense then, since otherwise we'd need to statically link the
full libromdata.a, but now that it's a shared library, we can just
link libromdata.so and avoid the forwarder nonsense.
ExtractorPlugin no longer needs libdl, since it doesn't use dlopen().
Likewise, the factory method has been removed, since it was only used
by the forwarder plugin.
The ExtractorPlugin shared library now has two .cpp files:
- ExtractorPlugin.cpp
- RpQUrl.cpp
RpQUrl is normally in rom-properties-(kde4|kf5|kf6), but it's small
enough that we can compile it for the plugin separately and statically
link it in without too much overhead.
Split the RomPropertiesKDE namespace redefinition macros into RpQtNS.hpp.
This lets us include it without including the whole RpQt.hpp.
(RpQt.hpp includes RpQtNS.hpp for compatibility purposes.)
TODO: Do the same for OverlayIconPlugin.
Also, verify plugin size differences.
NOTE: Conditional inclusion for Qt5 vs. Qt6 JSON doesn't work properly.
Automoc complains, and the build fails:
AutoMoc warning:
"src/kde/ExtractorPlugin.hpp"
Could not find dependency file "kf6/ExtractorPlugin.json"
To fix this, we need separate header files for KF5 and KF6.
Hence, ExtractorPluginKF5.hpp and ExtractorPluginKF6.hpp.
Add #include <QtCore/QObject> to the ExtractorPlugin*.hpp files,
since the KF6 build is complaining that QObject isn't defined.
(This replaces the qglobal.h include.)
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, no LTO)
text data bss dec hex filename
595596 22584 80 618260 96f14 rom-properties-kf5.so [before]
588702 22272 80 611054 952ee rom-properties-kf5.so [after]
-6894 -312 0 -7206 -1c26 Difference
text data bss dec hex filename
15925 1088 48 17061 42a5 kfilemetadata_rom-properties-kf5.so [before]
29230 1600 48 30878 789e kfilemetadata_rom-properties-kf5.so [after]
+13305 +512 0 +13817 +35f9 Difference
Code size difference: (64-bit Gentoo Linux, gcc-13.2.0, release build, with LTO)
text data bss dec hex filename
542255 22568 64 564887 89e97 rom-properties-kf5.so [before]
535457 22224 72 557753 882b9 rom-properties-kf5.so [after]
-6798 -344 +8 -7134 -1bde Difference
text data bss dec hex filename
15853 1088 40 16981 4255 kfilemetadata_rom-properties-kf5.so [before]
27856 1576 40 29472 7320 kfilemetadata_rom-properties-kf5.so [after]
+12003 +488 0 +12491 +30cb Difference
Not sure why the size increase in the kfilemetadata plugin is almost
double the decrease in the main plugin... (LTO might help?)
I'm planning on reworking the "forwarder" plugins to not be forwarders,
since libromdata.so makes that unnecessary. openQUrl() is needed by the
two plugins, so move it to RpQUrl.cpp to reduce the number of files
that will be needed in the plugins.
loadFieldData(), and later loadMetaData(), will make use of the
already-loaded BAM/header instead of loading it again.
remove_A0_padding(): Don't write to `buf` anymore. Instead, return the
actual string length without the A0 padding. This allows us to use the
function without modifying the stored BAM/header.
TODO: Maybe we should switch to application/vnd.commodore.* ?
[xdg] rom-properties.xml: Remove a duplicate D71 definition.
Added aliases for the old MIME types, plus:
- application/x-c64-datadisk (D64)
- application/x-c64-rawdisk (G64)
Ignoring the NZ flag for JR NZ, dd.
Not adding some opcodes that make no sense for an interrupt vector, e.g.
Blockade Runner (NA) has "F5 C5 D5" (PUSH AF; PUSH BC; PUSH DE).
