Linux: enable HTML5 and Android builds, fix display.save() and network.requestId - #936
Open
chkuendig wants to merge 6 commits into
Open
Linux: enable HTML5 and Android builds, fix display.save() and network.requestId#936chkuendig wants to merge 6 commits into
chkuendig wants to merge 6 commits into
Conversation
|
|
chkuendig
force-pushed
the
linux-builder-fixes
branch
from
August 15, 2026 12:21
0f268c8 to
39e3789
Compare
chkuendig
marked this pull request as draft
September 1, 2026 19:51
The packager is already compiled in; only CORONABUILDER_HTML5 was missing, so the binary answered that HTML5 builds are not supported on this operating system. A build still needs a real webtemplate.zip. It is produced by the emscripten target and reaches a packaged build as a CI artifact, so the copy in this tree is an empty placeholder and the packager stops at "Failed to open template".
Defines CORONABUILDER_ANDROID and adds the Rtt_AndroidSupportTools.c the factory calls from inside that gate. GetResourceDirectory() gains a Linux arm. Under Rtt_LINUX_ENV it matched no branch and ran off the end of a function returning const char *. AndroidValidation.lua is installed with it: the packager loads that script from disk with luaL_loadfile rather than through LUA_SOURCES, and stops at "Could not find script file" without it. GetStartupPath() checks readlink() before terminating the buffer. On failure it returned -1 and the terminator was written one byte before the buffer. Android builds also need android-template.zip and Corona.aar. Both are built by platform/android/app/build.gradle.kts as part of Corona Native, which the Linux cmake build does not invoke; a packaged build receives them as a CI artifact.
The four sandbox paths start as empty strings, so appending alone pointed them at /Documents, /TemporaryFiles, /CachedFiles and /.system - the root of the filesystem, which only root can write. Each run now gets its own directory under TMPDIR, the same shape as the per-process temporary directory the Windows builder creates, and it is removed when the run finishes. mkdtemp makes it atomically with mode 0700; on Linux TMPDIR is usually the shared /tmp.
On Linux a captured frame is tagged kBGRA. Both writers read that name as the order the bytes sit in, so savePNG turned a captured pixel into G,R,A,B and saveJPG into G,R,A: red and green swapped, blue taken from the alpha byte. That is every display.save() on Linux. The capture is read back as GL_BGRA with a packed 8_8_8_8 type, so its bytes are A,R,G,B. kRGB and kRGBA keep their existing byte order: the shared loaders and the GLES capture Emscripten uses fill those buffers literally, so reordering them would corrupt what they hand over. The format names are not used consistently across the engine, so each case states the layout it is actually given. Formats no caller produces are rejected rather than misread. saveJPG in particular used to run a mask or luminance-alpha buffer through a three-byte row stride. saveJPG's conversion buffer came from malloc but was held in a std::unique_ptr whose deleter calls delete. Both writers now allocate with new[] and let the pointer free it.
NetworkRequestState carried an fRequestID that nothing ever assigned and pushToLuaState never read, so event.requestId came back nil on Linux while network.request() had already handed the caller an id. Listeners that key their bookkeeping off the id — the pattern the mac and win32 backends support — had no way to match a response to its request. The id now travels from the request parameters onto the state the event is built from, and lands in the event table between "url" and "bytesTransferred", the slot AppleNetworkSupport.mm and WindowsNetworkSupport.cpp put it in. Those two push a RequestCanceller object; Linux keeps the plain number it already returns from network.request() and accepts in network.cancel(). fRequestID becomes unsigned to match the counter it now carries: the ids originate as unsigned int in NetworkRequestParameters and are keyed that way in the request map, so a signed field would have made the event disagree with the returned value once the counter passed INT_MAX. cancel() converted argument 1 with lua_tonumber and used the result straight away as a key into the request map. That function answers 0 for a missing argument, nil, false, a table or a non-numeric string, and truncates a fractional one, so network.cancel(), network.cancel(nil) and network.cancel(1.9) all named a request the caller never meant to cancel — and returned true once they found it. Negative, infinite and out-of-range values reached an undefined conversion to unsigned. The argument is now required to be a Lua number holding a finite, integral value inside the range the ids are allocated from, and only then converted. Anything else is reported through paramValidationFailure and nothing is pushed, matching what the win32, apple and android backends do with a handle they do not recognise. paramValidationFailure's declaration is corrected to const char * so callers outside NetworkSupport.cpp can reach the definition that is already written that way.
display.save() clamps a jpegQuality between 0 and 1 and hands it to SaveBitmap, but the Linux path dropped it: LinuxBaseBitmap::SaveBitmap took no such parameter, so LinuxPlatform::SaveBitmap discarded the argument it was given and a literal 75 went to saveJPG. That 75 is scaled by 100 and clamped, so every save asked for quality 100 regardless of what the caller wanted. saveJPG then set the quality before jpeg_set_defaults(), which resets it to 75. The value is now applied after the defaults, so it survives to the encoder.
chkuendig
changed the title
chkuendig
force-pushed
the
linux-builder-fixes
branch
from
September 2, 2026 21:35
39e3789 to
edda932
Compare
chkuendig
marked this pull request as ready for review
September 2, 2026 21:42
Author
|
@Shchvova Updated this as the initial version had some issues. Let me know if there's anything to change/clarify. I have a feeling the linux path has not been been very much exercised in the past, but having it for automations (Agentic AI or just CI/CD) it's super useful. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I recently started using Solar2D in Docker (mostly for Github Action automations and to use solar2d-mcp and noticed a few issues:
build fixes:
CORONABUILDER_HTML5 was never defined for the Linux build, so Solar2DBuilder rejected html5 as an unknown platform. Defining it wires up the existing HTML5 packager that was already compiled in.
Same shape: CORONABUILDER_ANDROID undefined, plus
AndroidValidation.luawasn’t copied next to the builder, so the packager couldn’t find it at runtime. This supersedes fix(linux): enable Solar2DBuilder Android builds on Linux #891The four sandbox paths started as empty strings, so appending gave
/Documents,/TemporaryFiles,/CachedFilesand.system— the filesystem root, writable only by root. Each run now gets its ownmkdtempdirectory under TMPDIR (atomic, mode 0700), removed when the run ends, matching the per-process directory the Windows builder already creates.screenshot fixes
A Linux capture is tagged kBGRA, and both writers read that name as the byte order. The capture is actually A,R,G,B, so savePNG emitted G,R,A,B and saveJPG emitted G,R,A — red and green swapped, blue taken from the alpha byte. Also rejects formats no caller produces rather than misreading them, and fixes a
malloc/unique_ptr-deleter mismatch.LinuxBaseBitmap::SaveBitmaphad no quality parameter, soLinuxPlatform::SaveBitmapdiscarded the caller’s value and passed a literal75into an API expecting 0–1 andsaveJPGset quality beforejpeg_set_defaults, which resets it.network fixes
fRequestIDwas never assigned and never read, soevent.requestIdcame back nil even thoughnetwork.request()had handed the caller an id — listeners had no way to match a response to its request. The id now lands between "url" and "bytesTransferred", the slot the Apple and Windows backends use, and the field is unsigned to match the counter.Separately,
network.cancel()fed lua_tonumber straight into the request map: a missing or invalid argument all became 0 and could cancel a request the caller never named, returning true. The argument is now validated as a finite integral number in range.