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

Turn Transition to a Drawable; Implement missing transitions (RandomBlockUp/Down, WaveIn/Out, ZoomIn/Out, MosaicIn/Out) #1369

Merged
merged 4 commits into from Jul 1, 2018

Conversation

Projects
None yet
4 participants
@Albeleon
Member

Albeleon commented May 30, 2018

Implemented Transitions (RandomBlockUp/Down, WaverIn/Out, ZoomIn/Out, MosaicIn/Out)

Implementation of the transitions left:

-RandomBlockUp/Down: This is made like Random, defining the order of blocks when the transition is initiated, and then Update the same way, so they share that code. The inicialization however is more complex to create an effect both incremental and random:

   · First we need to reorder our array incrementally (index 0 is 0, index 1 is 1, etc.), so we have an order established. In RandomBlocksUp we reverse the order.

  · The progress in the array is marked by a new vertical line to print, and it prints point up until 10 vertical lines later. So, the best way to print with this semi-random incremental order is:
            1 > We shuffle the array with the first 10 lines (600 in this case).
            2 > The first 80 (number of block per vertical line) will be fixed.
            3 > We shuffle the array from the position after the first 80, and we shuffle another 10 position lines.
            4 > Continue until there are no next 10 lines to print. This way, we've given a priority to the lines from one direction and the ones from the other direction won't have the opportunity to be printed until later.

   · This effect by itself replicates fine the effect, but there are some weird details that need a little more code to solve:
            - When it starts, the first 10 lines have the same probability of having random pixels, so the first 3 or so lines don't have more than them. To solve it, during the first iterations the number of lines where it is allowed to be printed is not the next 10, but 1, 3, 5, etc. That way it starts smoother.
            - When it ends, some blocks left can still be hanging on to the end in the beginning of the transition, which makes it jarring when they suddenly disappear. To reduce those ones, we use right after the shuffle a partial_sort function to put in each iteration a couple of the lowest blocks first in the order to print, so progressively the ones at the bottom get eliminated enough to mitigate the jarring effect. There are some times it doesn't print the lowest of all of them or it skips some test to not feel like there's an automatic drawer working behind.

   · The complexity of the Transition inicialization is (w = width, h = height, b = block size (4)): for [h] * (shuffle [w * h / b^2] + partial_sort [[w * h / b^2] * (log(cte) ~ 1)]) => 2 * w * h^2 / b^4 (with our data => 144.000) . The Transition Update is: for [w * h / b^2] (with our data => 4800).

   · POSSIBLE ALTERNATIVE TO RandomBlock Update: Instead of printing in each Update the number of blocks, we can use a Bitmap Mask where each frame we insert the corresponding blocks, and then mask Screen1 with this Mask. We would only use a few blits to insert new blocks in the Mask, and a BlitMask, instead of doing O(w * h / b^2) blits.

-ZoomIn/Out: This transition zooms in and out of the hero's position, and needs some adjustments to properly reflect the zoom of the RPG Maker:

   · First, we create a zoom_position vector parameter that is set when the transition is initialized. If the current scene is a map (there is a hero character), it returns their screen position. If it's not, it returns the center of the screen (160, 120).

   · In the update, we use StretchBlit using the percentage as the progress of the position and the extension of the zoom.

   · Because ZoomIn and Out are similar but with the percentage in reverse and a different screen pointer, to save code we make them one and the same, and when it's ZoomOut we reverse the percentage and change the screen pointer from one to the other.

   · This zoom is not enough to replicate the effect. The way the previous code would work, it would zoom on the character, but its relative position on the screen remains the same (if the character is 90% right of the screen, even with zoom it will remain there). In RM2000, a character too far off the center will have a zoom that will be next to the closest border until the character is closed enough (for our cases, when it's in between 25% and 50% of the screen), and then it continues the normal zoom. So all those variables are to take in consideration this behaviour.

   · Related to the one above, X and Y act independently this time, and therefore to simplify code and variables we can use vectors to reflect their positions, and use a for to make the same operations for both.

   · The complexity is one StretchBlit per Update.
  • MosaicIn/Out: This transition blurs the screen. It is blurred incrementally (the size of each block/mosaic grows linearly according to the percentage, starting from size 1 and ending in size 40). It doesn't need initialization, only Update:

     · We cannot just scale down, and then up the screen to have the blurred effect because:
              1 > When the width and height size isn't divisible by the current mosaic size ( w|h % size != 0 ), because it's an integer, pixels are lost, so when it's scaled up again it doesn't cover the whole screen.
              2 > When the mosaic size starts getting higher and higher, given the low number of width and height (320x240), some adjacent sizes give the same result when you divide the parameters with them (320 / 31 = 10 + m; 320 / 32 = 10), so the screen will not change the colors of each pixel and it will look like the transition froze for a moment.
    
     · Even though it's not very efficient, so far we have to print each individual block. For each size, we print a block that takes the first pixel inside (upper-left) and colors that part with FillRect (StretchBlit has some issue that I will post in the Issues section). I hope FillRect is more efficient than StretchBlit too. We access the color of that pixel position with a static_cast and the int32_t to rgba function.
    
     · To not give the feeling that the mosaics are too disorienting, nor too obvious they grow from a pivot point in the upper-left corner, we move their offset with the operation (w / 2) % mosaic_size (and same with the y position and height) so they are more or less focused on the center and their alteration is a right balance between not too jumpy and not too mechanic.
    
     · Just like ZoomIn/Out, both versions of the Mosaic Transition are the same except the Screen pointer and the reverse percentage, so we do the exact same to save code.
    
     · The complexity of the code is the highest so far (w = width, h = height, m = mosaic size): for [w / m] * for [h / m] = w * h / m^2 FillRects (with our data, for the lowest size (m = 2) => 19.200. It would be good to find a more efficient code than this.
    

-WaverIn/Out: This transition dissolved the screen like a wave. There is already a perfectly workable function to create Wave effects (WaverBlit), so we just use it with different parameters until we find the one that is the closest to the Wave effect in RM2000. The complexity is one WaverBlit (which inside it uses one Blit per line (h = 240)) each Update.

Fix #900
Fix #1094

@Ghabry

This comment has been minimized.

Member

Ghabry commented May 30, 2018

When the FPS layer is active it is included in the transition effects.
This is a problem with all effects that move stuff around and should be fixed as part of #1094

AUTHORS.md Outdated
@@ -17,3 +17,4 @@ EasyRPG Player authors
* scurest
* Shin-NiL
* Takeshi Watanabe (takecheeze)
* Alberto León Meaños (albeleon)

This comment has been minimized.

@Ghabry

Ghabry May 30, 2018

Member

Please respect the alphabetical order.

You will be the first in the list, congratulation

@Ghabry

This comment has been minimized.

Member

Ghabry commented May 31, 2018

About your question in the chat:

"old_instances" is only filled when "Pop" is used to remove scenes from the scene stack.

There is "Scene::Find" for finding a specific scene but this won't help you here because the scene will be also on the stack while IN a battle -> incorrect zoom.

One possible option is to replace
void Graphics::Transition(TransitionType type, int duration, bool erase = false)
with
void Graphics::Transition(TransitionType type, int duration, bool erase = false, int x_pos = -1, int y_pos = -1)
and then modify the invocation of the Transition functions everywhere when appropriate.

This is not ideal because is only used for zoom but I don't know a better way.

@Albeleon

This comment has been minimized.

Member

Albeleon commented May 31, 2018

Oh, I forgot to put in the commit, I modified the Game_Map to add in ScrollHorizontal and Vertical whether the map is already defined or not, because if it's not that function could be invoked and give an error. This happens if you use game_player.GetScreenX/Y(), for example, in a title, when game_player has been defined but no map has yet.

@carstene1ns

Needs some style fixes.

if (Input::IsTriggered(Input::SHOW_LOG)) { Output::ToggleLog(); }
if (Input::IsTriggered(Input::RESET)) { reset_flag = true; }
if (Input::IsTriggered(Input::TOGGLE_ZOOM)) { DisplayUi->ToggleZoom(); }
if (Input::IsTriggered(Input::TOGGLE_FULLSCREEN)) { DisplayUi->ToggleFullscreen(); }

This comment has been minimized.

@carstene1ns

carstene1ns Jun 4, 2018

Member

Please do not change code formatting carelessly.
(As it unrelated to the implemented feature here)

@@ -757,4 +759,4 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
</ImportGroup>
</Project>
</Project>

This comment has been minimized.

@carstene1ns

carstene1ns Jun 4, 2018

Member

need to update the .filters as well. missing newline.

src/utils.h Outdated
* Gets the Random Number Generator (RNG) from Utils.
*
* @return the random number generator
*/
std::mt19937 &GetRNG();

This comment has been minimized.

@carstene1ns

carstene1ns Jun 4, 2018

Member

this file changed upstream. you need to update the branch you are rebasing on.

@Albeleon

This comment has been minimized.

Member

Albeleon commented Jun 4, 2018

I forgot to say it, but now the transitions are also compatible with TransitionNone (when in Default Teleport you put "Not erased" as an option). Here the result: https://i.imgur.com/F2KJBdI.mp4

@Albeleon

This comment has been minimized.

Member

Albeleon commented Jun 5, 2018

Small update fixing a few bugs in the Zoom transition and TransitionNone + inmediate transition. Now the black flash on the video I linked before in the last transition doesn't show up.

@Ghabry

Just code style review.
Had no time yet to review your changes (especially for Player::Update and Graphics.cpp I need some additional time & tests to understand that refactoring :).

if (Input::IsTriggered(Input::TOGGLE_FULLSCREEN)) {
DisplayUi->ToggleFullscreen();
}
}

This comment has been minimized.

@Ghabry

Ghabry Jun 6, 2018

Member

Not sure if github renders this incorrectly but the new indentation looks wrong.

*
* You should have received a copy of the GNU General Public License
* along with EasyRPG Player. If not, see <http://www.gnu.org/licenses/>.
*/

This comment has been minimized.

@Ghabry

Ghabry Jun 6, 2018

Member

As usual the stars * are incorrect. They need one extra whitespace to align correctly.

/*
* Renders the frame overlay.

This comment has been minimized.

@Ghabry

Ghabry Jun 6, 2018

Member

bogus documentation comment

* @param type transition type.
* @param linked_scene scene transitioning, it should be either the scene summoning (this) or the current instance (Scene::instance.get())
* @param duration transition duration.
* @param erase erase screen flag.

This comment has been minimized.

@Ghabry

Ghabry Jun 6, 2018

Member

again the comment whitespace in the whole file

}
void Graphics::Update() {
//FPS:
if (next_fps_time == 0) { next_fps_time = DisplayUi->GetTicks() + 1000; }

This comment has been minimized.

@Ghabry

Ghabry Jun 6, 2018

Member

use proper formating:
if () {
// code
}

@Ghabry Ghabry added the Refactor label Jun 6, 2018

/**
* Transition class.
* Shows the transition between two screens.
*/

This comment has been minimized.

@carstene1ns

carstene1ns Jun 7, 2018

Member

^ forgot these.

@Albeleon

This comment has been minimized.

Member

Albeleon commented Jun 9, 2018

I link #1125 because maybe with the refactor this has been affected. Right now I can't test, later I'll try to.

current_scene->DrawBackground();
if (state.drawable_list.size())

This comment has been minimized.

@Ghabry

Ghabry Jun 18, 2018

Member

!drawable_list.empty()

@@ -495,3 +217,7 @@ int Graphics::GetDefaultFps() {
MessageOverlay& Graphics::GetMessageOverlay() {
return *message_overlay;
}
Transition *Graphics::GetTransition() {

This comment has been minimized.

@Ghabry

Ghabry Jun 18, 2018

Member

Is it possible that Transition is nullptr? If not you can do it like MessageOverlay above and return by & reference.

@@ -198,6 +198,7 @@ void Player::Init(int argc, char *argv[]) {
void Player::Run() {
Scene::Push(std::shared_ptr<Scene>(static_cast<Scene*>(new Scene_Logo())));
Graphics::UpdateSceneCallback();

This comment has been minimized.

@Ghabry

Ghabry Jun 18, 2018

Member

is this a bugfix or unnecessary?

This comment has been minimized.

@Ghabry

Ghabry Jun 18, 2018

Member

yeah is a bugfix, segfaults without this line

@Ghabry

This comment has been minimized.

Member

Ghabry commented Jun 18, 2018

When choosing Exit in the Title Screen I get a segfault because transision is null. (was already deleted in Graphics::Quit)

Transition::IsActive transition.cpp:343
Graphics::IsTransitionPending graphics.cpp:171
Player::Run player.cpp:221
main main.cpp:28
__libc_start_main 0x00007ffff111806b
_start 0x00005555556ec33a
@Ghabry

Ghabry approved these changes Jun 18, 2018

Works and looks fine. Thanks for doing such a huge contribution directly at the beginning of your "EasyRPG Dev Career". Looking forward to more. :)

@Ghabry

Ghabry approved these changes Jun 23, 2018

@Ghabry

This comment has been minimized.

Member

Ghabry commented Jun 24, 2018

Could you make some changes to the commit list? (rebase and set all commits to "edit")

  • Transition change: … Change: Sum up the whole change in the first line, e.g. "Make Transition a Drawable and implement missing transitions (RandomBlockUp/Down, WaverIn/Out, ZoomIn/Out, MosaicIn/Out)"
  • Style/Formatting errors corrected (Edit: carstene1ns' also corrected) Just style fixes Squash into previous one
  • Fix Issue ( #1371 ): "4.2: RM2000 + RM2003: There should be white fla… …
  • Format fixes and Checking in Graphics::UpdateSceneCallback whether Tr… … Changes: You probably mean IsTransitionPending not UpdateSceneCallback
  • Updating "4.2: RM2000 + RM2003: There should be white flashes when a … … Changes: State in the first line what was changed e.g. you have this sentence later "When the Transition to a Battle was TransitionNone (appending those flashes), there was some weird behavior."
  • Update: When the Transition to a Battle was TransitionNone (appending those flashes), there was some weird behavior. These changes have been made to make it compatible and correct. Changes: Remove the "Update:". In General please don't prefix stuff like "Update:" or "PR:" and similiar in front of the messages.
  • New Update: RandomBlock Draw modification. … Change: Reword it e.g. to "Improve RandomBlock Draw performance"

@Albeleon Albeleon changed the title from Transitions (RandomBlocksDown/Up, ZoomIn/Out, MosaicInOut, WaveInOut) to Turn Transition to a Drawable; Implement missing transitions (RandomBlockUp/Down, WaveIn/Out, ZoomIn/Out, MosaicIn/Out) Jun 24, 2018

@Ghabry

Ghabry approved these changes Jun 24, 2018

@carstene1ns carstene1ns added this to the 0.5.4 milestone Jun 24, 2018

}
#endif
// Normal logic update
// Input:

This comment has been minimized.

@Ghabry
// Scene changed or webplayer waits for files.
// Not save to Update again, setup code must run
// Scene changed or webplayer waits for files. Not save to Update again, setup code must run:

This comment has been minimized.

@Ghabry

Albeleon added some commits Jun 4, 2018

"Turn Transition to a Drawable; Implement missing transitions (Random…
…BlockUp/Down, WaveIn/Out, ZoomIn/Out, MosaicIn/Out)

This Update fixes #1094 by turning Transition into a Drawable and separating Logic from Drawing. It makes the following changes:

    - It separates the functions related with Transitions in Graphics.h/cpp to its own files as Transition.h/cpp.
    - It turns Transition in a Drawable, as an attribute of Graphics that other files can access via Graphics::GetTransition(). It's a global drawable, it has an Update where the logic frames are updated (instead of in the Drawing function), and the TransitionUpdate is now the override Draw. It also adds in Drawable the DrawableType::TypeTransition, whose priority comes below Overlay but above Frame.
    - It changes every reference from other files from Graphics::Transition to Transition::Init to initiate the right transition. Same with the TransitionTypes.
    - It changes quite a few names to make it more consistent:
        · Graphics::DrawFrame -> Graphics::Draw
        · Graphics::Transition -> Transition::Init
        · Graphics::TransitionUpdate -> Transition::Update and Transition::Draw

    - It separates Local and Global drawing of Drawables, and when the screen is erased because a transition (easily checkable by Transition::IsErased()) it prints GlobalDraw, so the overlays should be printed when the screen is erased.
    - The function Graphics::SnapToBitmap, LocalDraw and GlobalDraw have an input argument (priority), which means it will print every Drawable that is equal or below it. If the input is not filled, it uses TransitionType::TransitionMaximum, which means it prints all of them.
    - Also, when a transition is summoned and takes a frozen screen, it makes sure it only prints the drawables below Transition.
    - Graphics::IsTransitionPending now checks if Transition exists before checking if it's active. This is done since, when they Graphics are reseted when they are eliminated, this function is still called one more time.

    - Now, Update and Draw are completely independent. Graphics::Update only updates logic (except FPS), and when it's time to draw, Player::Update uses Graphics::Draw. Graphics::Update updates the FPS and Transition, and it's always summoned in the logic Update regardless of whether the Scene need updating or not. It's also inside the speed_modifier, so fast_forward affects these Updates.

IMPORTANT: Now that the transition frames are tied to Update logic instead of when they're drawn in the graphics, that means they are affected by lags. This keeps the FPS correct but graphically it can get laggy if the functions have a lot of delay factor. Some functions that have been checked to be really slow and cause lag are Game_System::BgmPlay, Game_System::BgmStop or Player::SetupPlayerSpawn. Which means there will be some lags when starting a new game when there's music, or when loading a file.

Also, Transitions RandomBlocksUp/Down, ZoomIn/Out, MosaicIn/Out and WaveIn/Out have been implemented:
 - RandomBlocksUp/Down work like RandomBlocks but with a different order.
 - ZoomIn/Out uses StretchBlit in Draw(), saving beforehand the character's screen position where to zoom. The zoom isn't direct, it only starts zooming right directly on the character when they are in the 75% inner screen, so a few changes were made to be as similar as RPG_RT.
 - MosaicIn/Out uses FillRect after taking each pixel. When mosaic_size = 2, 3 or 4, it's the most heavy performance wise (320 / size * 240 / size), but the bigger the mosaic_size is, the less time it takes. Also, the pixel and position it takes is modified a little to not create confusing chaos or intuitive order.
 - WaveInOut uses WaverBlit with the right parameters to imitate RPG_RT's transition.
Fix Issue ( #1371 ): "4.2: RM2000 + RM2003: There should be white fla…
…shes when a battle starts between the battle sound and the TransitionOut. If we look the video in slow motion we can see it's not 1 flash, but two."

Solution: Add an Transition::AppendBefore function summoned right after the Transition::Init in Scene_Map::TransitionOut when there is a battle. AppendBefore has as arguments the color of the flash, the frame duration of each flash, and the number of iterations. The modifications make that when the Transition is drawn, first it generates those flashes in the frozen screen, and then it starts the selected transition.
Improve RandomBlock Draw performance.
Problem: When Drawing the RandomBlockFunction, by the end of the function it was still in a "for" with 4800 Blit operations, which could be costly the more they were printed.

Solution: Create a BitmapRef called random_block_transition = transparent and an Int called current_blocks_print = 0. Each time the Draw function is called for a RandomBlockTransition, it checks the number of blocks to be printed from the previous time Draw was called to this one, and they are printed in random_block_transition (which means, in cases of constant low Draws, it only has to print a few Blits). Then the first screen and the second random_block_transition screen is printed. It helps performance during the Draw operation.
Adding quick TransitionIn to map after Menu/Save/Name/Shop/Load calling
Problem: "The transition between one of those menus to the previous map should be a quick FadeIn, but it fades in with the default Scene_Map.

Solution: Add a "transition_menu" bool in Game_Temp that is activated everytime a menu is called and checked in Scene_Map::TransitionIn, where it's deactivated and it calls Scene::TransitionIn (quick fadein).
@carstene1ns

Transitions work now as expected.

@carstene1ns carstene1ns merged commit b1259be into EasyRPG:master Jul 1, 2018

7 checks passed

Android (armeabi-v7a) Build finished.
Details
GCW0 Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details

@Albeleon Albeleon deleted the Albeleon:Transitions branch Jul 1, 2018

@Albeleon Albeleon referenced this pull request Jul 12, 2018

Open

Ascendence: Visual bugs #1293

2 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment