From c04dbcb19a11cae6ec7cd90d797c52bad3ca1f40 Mon Sep 17 00:00:00 2001 From: Yves Rizoud Date: Tue, 26 Jan 2010 02:23:44 +0000 Subject: [PATCH] Layer-specific: Fixed possible double free on exit. Removed unused global pointer. Implemented proper backup (history) before the various Copy-to-spare actions, fixing a severe history bug when using layers (issue 270). Grafx2 no longer asks 'Spare page was modified, continue?' on Spare modifications. Fixed missing screen refresh in Copy-to-spare (issue 293). Fixed the save-on-crash to actually save the spare git-svn-id: svn://pulkomandy.tk/GrafX2/trunk@1282 416bcca6-2ee7-4201-b75f-2eb2f807beb1 --- buttons.c | 53 +++++++++++++++--------- global.h | 2 - loadsave.c | 2 +- main.c | 4 -- misc.c | 2 +- pages.c | 116 ++++++++++++++++++++++++++++++++++++++++++++--------- pages.h | 6 +++ 7 files changed, 139 insertions(+), 46 deletions(-) diff --git a/buttons.c b/buttons.c index dc6a5c3b..e3709d99 100644 --- a/buttons.c +++ b/buttons.c @@ -1332,6 +1332,7 @@ void Copy_image_only(void) if (Backup_and_resize_the_spare(Main_image_width,Main_image_height)) { byte i; + for (i=0; iPages->Nb_layers; i++) { if (i == Spare_current_layer) @@ -1381,6 +1382,17 @@ void Copy_image_only(void) Spare_separator_position=Main_separator_position; Spare_X_zoom=Main_X_zoom; Spare_separator_proportion=Main_separator_proportion; + + // Update the visible buffer of the spare. + // It's a bit complex because at the moment, to save memory, + // the spare doesn't have a full visible_buffer + depth_buffer, + // so I can't use exactly the same technique as for Main page. + // (It's the same reason that the "Page" function gets complex, + // it needs to rebuild a depth buffer only, trusting the + // depth buffer that was already available in Spare_.) + Update_spare_buffers(Spare_image_width,Spare_image_height); + Redraw_spare_image(); + } else Message_out_of_memory(); @@ -1396,9 +1408,10 @@ void Copy_some_colors(void) memset(mask_color_to_copy,1,256); Menu_tag_colors("Tag colors to copy",mask_color_to_copy,&confirmation,0, NULL, 0xFFFF); - if (confirmation && - (!Spare_image_is_modified || Confirmation_box("Spare page was modified. Proceed?"))) + if (confirmation) { + // Make a backup with the same pixel data as previous history steps + Backup_the_spare(0); for (index=0; index<256; index++) { if (mask_color_to_copy[index]) @@ -1442,27 +1455,31 @@ void Button_Copy_page(void) if (clicked_button!=6) { if (clicked_button==4) + { + // Will backup if needed Copy_some_colors(); + } else { - if ( (!Spare_image_is_modified) - || (Confirmation_box("Spare page was modified. Proceed?")) ) + if (clicked_button<=2) { - // FIXME: add here some code to backup the spare - - if (clicked_button<=2) - Copy_image_only(); - - if (clicked_button==5) - Remap_spare(); - - if (clicked_button!=2) // copie de la palette - memcpy(Spare_palette,Main_palette,sizeof(T_Palette)); - - // FIXME: here is the 'end_of_modifications' for spare. - - Spare_image_is_modified=1; + Backup_the_spare(-1); + Copy_image_only(); } + else + Backup_the_spare(0); + + if (clicked_button==5) + Remap_spare(); + + if (clicked_button!=2) // copie de la palette + memcpy(Spare_palette,Main_palette,sizeof(T_Palette)); + + // Here is the 'end_of_modifications' for spare. + Update_spare_buffers(Spare_image_width,Spare_image_height); + Redraw_spare_image(); + + Spare_image_is_modified=1; } } diff --git a/global.h b/global.h index 053d5859..07a420c4 100644 --- a/global.h +++ b/global.h @@ -352,8 +352,6 @@ GFX2_GLOBAL byte Main_safety_backup_prefix; // -- Spare page data -/// Pointer to the pixel data of the spare page -GFX2_GLOBAL byte * Spare_screen; /// Palette of the spare page GFX2_GLOBAL T_Palette Spare_palette; /// Boolean, means the spare page has been modified since last save. diff --git a/loadsave.c b/loadsave.c index 35dc1ae1..18f2d2dc 100644 --- a/loadsave.c +++ b/loadsave.c @@ -988,7 +988,7 @@ void Image_emergency_backup() if (Main_backups && Main_backups->Pages && Main_backups->Pages->Nb_layers == 1) Emergency_backup("a999999.bkp",Main_screen, Main_image_width, Main_image_height, &Main_palette); if (Spare_backups && Spare_backups->Pages && Spare_backups->Pages->Nb_layers == 1) - Emergency_backup("b999999.bkp",Spare_screen, Spare_image_width, Spare_image_height, &Spare_palette); + Emergency_backup("b999999.bkp",Spare_visible_image.Image, Spare_image_width, Spare_image_height, &Spare_palette); } T_Format * Get_fileformat(byte format) diff --git a/main.c b/main.c index 64fadc6b..4d99b0dc 100644 --- a/main.c +++ b/main.c @@ -848,10 +848,6 @@ void Program_shutdown(void) free(Brush); Brush = NULL; Set_number_of_backups(0); - free(Spare_screen); - Spare_screen = NULL; - free(Main_screen); - Main_screen = NULL; // Free the skin (Gui graphics) data free(Gfx); diff --git a/misc.c b/misc.c index 69dbd3f6..cd7636b5 100644 --- a/misc.c +++ b/misc.c @@ -293,7 +293,7 @@ byte Read_pixel_from_spare_screen(word x,word y) // Clipping is required as this can be called with coordinates from main image // (can be a bigger or smaller image) if (x>=Spare_image_width || y>=Spare_image_height) - return 0; // TODO: we could return the spare's transparent color, if it has one. + return Spare_backups->Pages->Transparent_color; #ifndef NOLAYERS return *(Spare_visible_image.Image + y*Spare_image_width + x); #else diff --git a/pages.c b/pages.c index 8b732984..66b6509e 100644 --- a/pages.c +++ b/pages.c @@ -268,6 +268,52 @@ void Update_depth_buffer(void) Download_infos_backup(Main_backups); } +void Redraw_spare_image(void) +{ + #ifndef NOLAYERS + // Re-construct the image with the visible layers + byte layer; + // First layer + for (layer=0; layerPages->Nb_layers; layer++) + { + if ((1<Pages->Image[layer], + Spare_image_width*Main_image_height); + + // No depth buffer in the spare + //memset(Spare_visible_image_depth_buffer.Image, + // layer, + // Spare_image_width*Spare_image_height); + + // skip all other layers + layer++; + break; + } + } + // subsequent layer(s) + for (; layerPages->Nb_layers; layer++) + { + if ((1<Pages->Image[layer]+i); + if (color != Spare_backups->Pages->Transparent_color) // transparent color + { + *(Spare_visible_image.Image+i) = color; + //if (layer != Spare_current_layer) + // *(Spare_visible_image_depth_buffer.Image+i) = layer; + } + } + } + } + #endif +} + void Redraw_current_layer(void) { #ifndef NOLAYERS @@ -309,7 +355,6 @@ void Download_infos_page_spare(T_Page * page) { if (page!=NULL) { - //Spare_screen=page->Image[Spare_current_layer]; Spare_image_width=page->Width; Spare_image_height=page->Height; memcpy(Spare_palette,page->Palette,sizeof(T_Palette)); @@ -331,7 +376,6 @@ void Upload_infos_page_spare(T_Page * page) void Download_infos_backup(T_List_of_pages * list) { - //list->Pages->Next->Image[Main_current_layer]; if (Config.FX_Feedback) FX_feedback_screen=list->Pages->Image[Main_current_layer]; @@ -497,18 +541,19 @@ void Free_last_page_of_list(T_List_of_pages * list) int Create_new_page(T_Page * new_page, T_List_of_pages * list, dword layer_mask) { -// Cette fonction crée une nouvelle page dont les attributs correspondent à -// ceux de new_page (width,height,...) (le champ Image est invalide -// à l'appel, c'est la fonction qui le met à jour), et l'enfile dans -// list. - +// This function fills the "Image" field of a new Page, +// based on the pages's attributes (width,height,...) +// then pushes it on front of a Page list. if (list->List_size >= (Config.Max_undo_pages+1)) { - // On manque de mémoire ou la list est pleine. Dans tous les - // cas, il faut libérer une page. + // List is full. + // If some other memory-limit was to be implemented, here would + // be the right place to do it. + // For example, we could rely on Stats_pages_memory, + // because it's the sum of all bitmaps in use (in bytes). - // Détruire la dernière page allouée dans la Liste_à_raboter + // Destroy the latest page Free_last_page_of_list(list); } { @@ -790,11 +835,6 @@ int Backup_and_resize_the_spare(int width,int height) int return_code=0; byte nb_layers; - - // On remet à jour l'état des infos de la page de brouillon (pour pouvoir - // les retrouver plus tard) - Upload_infos_page_spare(Spare_backups->Pages); - nb_layers=Spare_backups->Pages->Nb_layers; // On crée un descripteur pour la nouvelle page de brouillon new_page=New_page(nb_layers); @@ -803,7 +843,10 @@ int Backup_and_resize_the_spare(int width,int height) Error(0); return 0; } - Upload_infos_page_spare(new_page); + + // Fill it with a copy of the latest history + Copy_S_page(new_page,Spare_backups->Pages); + new_page->Width=width; new_page->Height=height; if (Create_new_page(new_page,Spare_backups,0xFFFFFFFF)) @@ -819,6 +862,9 @@ int Backup_and_resize_the_spare(int width,int height) Download_infos_page_spare(Spare_backups->Pages); + // Light up the 'has unsaved changes' indicator + Spare_image_is_modified=1; + return_code=1; } return return_code; @@ -845,7 +891,7 @@ void Backup_layers(dword layer_mask) // retrouver plus tard) Upload_infos_page_main(Main_backups->Pages); - // On crée un descripteur pour la nouvelle page courante + // Create a fresh Page descriptor new_page=New_page(Main_backups->Pages->Nb_layers); if (!new_page) { @@ -853,14 +899,14 @@ void Backup_layers(dword layer_mask) return; } - // Enrichissement de l'historique + // Fill it with a copy of the latest history Copy_S_page(new_page,Main_backups->Pages); Create_new_page(new_page,Main_backups,layer_mask); Download_infos_page_main(new_page); Download_infos_backup(Main_backups); - // On copie l'image du backup vers la page courante: + // Copy the actual pixels from the backup to the latest page for (i=0; iPages->Nb_layers;i++) { if ((1<Pages->Next->Image[i], Main_image_width*Main_image_height); } - // On allume l'indicateur de modification de l'image + // Light up the 'has unsaved changes' indicator Main_image_is_modified=1; /* @@ -876,6 +922,36 @@ void Backup_layers(dword layer_mask) */ } +void Backup_the_spare(dword layer_mask) +{ + int i; + T_Page *new_page; + + // Create a fresh Page descriptor + new_page=New_page(Spare_backups->Pages->Nb_layers); + if (!new_page) + { + Error(0); + return; + } + + // Fill it with a copy of the latest history + Copy_S_page(new_page,Spare_backups->Pages); + Create_new_page(new_page,Spare_backups,layer_mask); + + // Copy the actual pixels from the backup to the latest page + for (i=0; iPages->Nb_layers;i++) + { + if ((1<Pages->Image[i], + Spare_backups->Pages->Next->Image[i], + Spare_image_width*Spare_image_height); + } + // Light up the 'has unsaved changes' indicator + Spare_image_is_modified=1; + +} + void Check_layers_limits() { if (Main_current_layer > Main_backups->Pages->Nb_layers-1) diff --git a/pages.h b/pages.h index 34510f96..09c3def6 100644 --- a/pages.h +++ b/pages.h @@ -88,6 +88,8 @@ void Free_page_of_a_list(T_List_of_pages * list); int Init_all_backup_lists(int width,int height); void Set_number_of_backups(int nb_backups); int Backup_with_new_dimensions(int upload,byte layers,int width,int height); +/// Backup the spare image, the one you don't see. +void Backup_the_spare(dword layer_mask); int Backup_and_resize_the_spare(int width,int height); /// Backup with a new copy for the working layer, and references for all others. void Backup(void); @@ -104,6 +106,10 @@ void Redraw_layered_image(void); void Redraw_current_layer(void); void Update_screen_targets(void); +/// Update all the special image buffers, if necessary. +int Update_buffers(int width, int height); +int Update_spare_buffers(int width, int height); +void Redraw_spare_image(void); /// /// STATISTICS