Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compression rewrite (results in a vc fix) #70

Open
wants to merge 288 commits into
base: old_main
Choose a base branch
from

Conversation

ariahiro64
Copy link
Collaborator

@ariahiro64 ariahiro64 commented Oct 1, 2023

I essentially did 4 things with this commit.

  1. Each compression algorithm uses its own function.

  2. Added zlib as a compression algorithm as it isnt terrible on a real control deck. This includes a resizable buffer.

  3. Every compression algorithm is forced to use Ofast which greatly improves decompression speed

  4. Rewrote the makefile which fixes Virtual Console

@ariahiro64 ariahiro64 added merge soon The pull request will be merged in 24 hours if nothing else comes up main branch The PR targets the master branch labels Oct 1, 2023
Copy link
Collaborator

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried it yet but I noticed few things in code

include/functions.h Outdated Show resolved Hide resolved
spec Outdated Show resolved Hide resolved
src/boot/lzo.c Outdated Show resolved Hide resolved
src/boot/z_std_dma.c Outdated Show resolved Hide resolved
src/boot/zlib.c Outdated Show resolved Hide resolved
@Yanis002 Yanis002 added waiting for author Waiting for the author to answer questions, do changes, ... and removed merge soon The pull request will be merged in 24 hours if nothing else comes up labels Oct 1, 2023
@Yanis002
Copy link
Collaborator

Yanis002 commented Oct 1, 2023

(also don't use merge soon label when you make the PR, use the waiting for review one instead and when it gets approved use merge soon)

@ariahiro64
Copy link
Collaborator Author

that should be the requested changes however we need to discuss the default memory size for zlib and perhaps use config.h?

@Yanis002
Copy link
Collaborator

Yanis002 commented Oct 1, 2023

maybe create a new file called config_compression.h so we can put compresison stuff in it, otherwise that solution seems good to me

Copy link
Collaborator

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is fine but I need to build this so I'll approve it once that is done

@ariahiro64
Copy link
Collaborator Author

in the process of applying changes but keep in mind this isnt tested on wiivc

@ariahiro64
Copy link
Collaborator Author

Just checked and Wii VC doesnt work even before this commit. So that will need to be addressed but that is out of the scope of this pr.

@Yanis002 Yanis002 added waiting for review Waiting for a contributor to review the pull request and removed waiting for author Waiting for the author to answer questions, do changes, ... labels Oct 1, 2023
Copy link
Collaborator

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as before I need to test before approving

/* Size of libdeflates's buffer in kibibytes (yes kib not kb). HackerSM64's implimentation uses a full Mb we use 32 kib*/
#define KIB 32

#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline missing there (at the end of the file)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also you should probably rename the KIB macro to something more specific to libdeflate, also there's a typo I missed on the comment there ("implimentation")

@Yanis002 Yanis002 added needs testing The pull request needs to be tested to make sure it works properly and removed waiting for review Waiting for a contributor to review the pull request labels Oct 1, 2023
@ariahiro64
Copy link
Collaborator Author

VC should now be fixed. Like stated VC doesn't benefit from optimization in fact it causes crashes. This should also be low maintenance for users. if people need insane compression the can just use zlib that's also in this pr. n64 mode uses the normal optimization levels so its mostly unchanged.

@ariahiro64
Copy link
Collaborator Author

Using make wad also forces VC mode as well, I did put a warning to make clean.

@ariahiro64 ariahiro64 changed the title compression rewrite compression rewrite (results in a vc fix) Oct 1, 2023
@Yanis002 Yanis002 added waiting for author Waiting for the author to answer questions, do changes, ... and removed needs testing The pull request needs to be tested to make sure it works properly labels Oct 13, 2023
@Yanis002
Copy link
Collaborator

I tried this PR everything seems to work, I requested changes I made and sent as a diff patch so I'm gonna wait until those are applied then I'll proceed with the merge process

Yanis002 and others added 6 commits January 9, 2024 12:49
* ``EMULATOR :=`` -> ``N64_EMULATOR ?=``

* change other instances of EMULATOR and removed EMU_FLAGS

* missing one EMULATOR
* Cleanup `z_collision_check.c` and structs

* Revert `other*` names to master, split to other pr

* WIP/experimental: `ColliderCylinderElement`

* Revert "WIP/experimental: `ColliderCylinderElement`"

This reverts commit cfc8c32ace2970869a2c93be0666786f87ca5e1a.

* ac/atHitInfo -> HitElem

* rename some collider elements to "elem" (instead of item, info, hurtbox...)

* cut down on more "hitbox" usage

* name all `ColliderElement*` temps properly

* rearrange colcheck structs

* add collider shape name descriptions

* reword collider shape descriptions

* jntsph first again

---------

Co-authored-by: fig02 <[email protected]>
* Removed unused imports and other minor improvements

* revert tools/ZAPD/ZAPD/genbuildinfo.py

* revert diff.py

* Update sym_info.py

* revert asm_processor/

---------

Co-authored-by: fig02 <[email protected]>
Co-authored-by: Dragorn421 <[email protected]>
* Replace most osSyncPrintf calls with PRINTF macro

* DEBUG -> OOT_DEBUG
…INT_LIGHTS` (#1583)

* 7 -> `ACTOR_FLAG_REACT_TO_LENS`

* move comment above actor flag

* 22 -> `ACTOR_FLAG_IGNORE_POINT_LIGHTS`

* newlines between flags

---------

Co-authored-by: fig02 <[email protected]>
@Yanis002 Yanis002 added 1.0 The PR targets HackerOoT v1.0 and removed main branch The PR targets the master branch labels Apr 21, 2024
@Yanis002
Copy link
Collaborator

Yanis002 commented Sep 7, 2024

what's going on with this pr lol the diff is suspicious

wouldn't it be better to remake it to target the current dev branch?

@ariahiro64
Copy link
Collaborator Author

this is a year old and may be quicker to redo than fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 The PR targets HackerOoT v1.0 waiting for author Waiting for the author to answer questions, do changes, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.