From 73fb445b8dbe88b1ac8e12acd2515618f9e64ac0 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 16 Aug 2020 11:32:45 +1000 Subject: Fix T78426: Header flips when changing editors The existing header flip detection didn't account for mixed tool-header and header flipping & visibility between space-types. Now alignment syncing handles any combination of header, tool-header & footer flipped state, in a way that can be extended to other region types in the future. --- source/blender/editors/screen/area.c | 268 ++++++++++++++++++++++++++--- source/blender/makesdna/DNA_screen_types.h | 4 + 2 files changed, 247 insertions(+), 25 deletions(-) diff --git a/source/blender/editors/screen/area.c b/source/blender/editors/screen/area.c index f87f631c643..296fe24edcd 100644 --- a/source/blender/editors/screen/area.c +++ b/source/blender/editors/screen/area.c @@ -2053,6 +2053,241 @@ void ED_area_data_swap(ScrArea *area_dst, ScrArea *area_src) SWAP(ListBase, area_dst->regionbase, area_src->regionbase); } +/* -------------------------------------------------------------------- */ +/** \name Region Alignment Syncing for Space Switching + * \{ */ + +/** + * Store the alignment & other info per region type + * (use as a region-type aligned array). + * + * \note Currently this is only done for headers, + * we might want to do this with the tool-bar in the future too. + */ +struct RegionTypeAlignInfo { + struct { + /** + * Values match #ARegion.alignment without flags (see #RGN_ALIGN_ENUM_FROM_MASK). + * store all so we can sync alignment without adding extra checks. + */ + short alignment; + /** + * Needed for detecting which header displays the space-type switcher. + */ + bool hidden; + } by_type[RGN_TYPE_LEN]; +}; + +static void region_align_info_from_area(ScrArea *area, struct RegionTypeAlignInfo *r_align_info) +{ + for (int index = 0; index < RGN_TYPE_LEN; index++) { + r_align_info->by_type[index].alignment = -1; + /* Default to true, when it doesn't exist - it's effectively hidden. */ + r_align_info->by_type[index].hidden = true; + } + + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + const int index = region->regiontype; + if ((uint)index < RGN_TYPE_LEN) { + r_align_info->by_type[index].alignment = RGN_ALIGN_ENUM_FROM_MASK(region->alignment); + r_align_info->by_type[index].hidden = (region->flag & RGN_FLAG_HIDDEN) != 0; + } + } +} + +/** + * Keeping alignment between headers keep the space-type selector button in the same place. + * This is complicated by the editor-type selector being placed on the header + * closest to the screen edge which changes based on hidden state. + * + * The tool-header is used when visible, otherwise the header is used. + */ +static short region_alignment_from_header_and_tool_header_state( + const struct RegionTypeAlignInfo *region_align_info, const short fallback) +{ + const short header_alignment = region_align_info->by_type[RGN_TYPE_HEADER].alignment; + const short tool_header_alignment = region_align_info->by_type[RGN_TYPE_TOOL_HEADER].alignment; + + const bool header_hidden = region_align_info->by_type[RGN_TYPE_HEADER].hidden; + const bool tool_header_hidden = region_align_info->by_type[RGN_TYPE_TOOL_HEADER].hidden; + + if ((tool_header_alignment != -1) && + /* If tool-header is hidden, use header alignment. */ + ((tool_header_hidden == false) || + /* Don't prioritize the tool-header if both are hidden (behave as if both are visible). + * Without this, switching to a space with headers hidden will flip the alignment + * upon switching to a space with visible headers. */ + (header_hidden && tool_header_hidden))) { + return tool_header_alignment; + } + if (header_alignment != -1) { + return header_alignment; + } + return fallback; +} + +/** + * Notes on header alignment syncing. + * + * This is as involved as it is because: + * + * - There are currently 3 kinds of headers. + * - All headers can independently visible & flipped to another side + * (except for the tool-header that depends on the header visibility). + * - We don't want the space-switching button to flip when switching spaces. + * From the user perspective it feels like a bug to move the button you click on + * to the opposite side of the area. + * - The space-switcher may be on either the header or the tool-header + * depending on the tool-header visibility. + * + * How this works: + * + * - When headers match on both spaces, we copy the alignment + * from the previous regions to the next regions when syncing. + * - Otherwise detect the _primary_ header (the one that shows the space type) + * and use this to set alignment for the headers in the destination area. + * - Header & tool-header/footer may be on opposite sides, this is preserved when syncing. + */ +static void region_align_info_to_area_for_headers( + const struct RegionTypeAlignInfo *region_align_info_src, + const struct RegionTypeAlignInfo *region_align_info_dst, + ARegion *region_by_type[RGN_TYPE_LEN]) +{ + /* Abbreviate access. */ + const short header_alignment_src = region_align_info_src->by_type[RGN_TYPE_HEADER].alignment; + const short tool_header_alignment_src = + region_align_info_src->by_type[RGN_TYPE_TOOL_HEADER].alignment; + + const bool tool_header_hidden_src = region_align_info_src->by_type[RGN_TYPE_TOOL_HEADER].hidden; + + const short primary_header_alignment_src = region_alignment_from_header_and_tool_header_state( + region_align_info_src, -1); + + /* Neither alignments are usable, don't sync. */ + if (primary_header_alignment_src == -1) { + return; + } + + const short header_alignment_dst = region_align_info_dst->by_type[RGN_TYPE_HEADER].alignment; + const short tool_header_alignment_dst = + region_align_info_dst->by_type[RGN_TYPE_TOOL_HEADER].alignment; + const short footer_alignment_dst = region_align_info_dst->by_type[RGN_TYPE_FOOTER].alignment; + + const bool tool_header_hidden_dst = region_align_info_dst->by_type[RGN_TYPE_TOOL_HEADER].hidden; + + /* New synchronized alignments to set (or ignore when left as -1). */ + short header_alignment_sync = -1; + short tool_header_alignment_sync = -1; + short footer_alignment_sync = -1; + + /* Both source/destination areas have same region configurations regarding headers. + * Simply copy the values. */ + if (((header_alignment_src != -1) == (header_alignment_dst != -1)) && + ((tool_header_alignment_src != -1) == (tool_header_alignment_dst != -1)) && + (tool_header_hidden_src == tool_header_hidden_dst)) { + if (header_alignment_dst != -1) { + header_alignment_sync = header_alignment_src; + } + if (tool_header_alignment_dst != -1) { + tool_header_alignment_sync = tool_header_alignment_src; + } + } + else { + /* Not an exact match, check the space selector isn't moving. */ + const short primary_header_alignment_dst = region_alignment_from_header_and_tool_header_state( + region_align_info_dst, -1); + + if (primary_header_alignment_src != primary_header_alignment_dst) { + if ((header_alignment_dst != -1) && (tool_header_alignment_dst != -1)) { + if (header_alignment_dst == tool_header_alignment_dst) { + /* Apply to both. */ + tool_header_alignment_sync = primary_header_alignment_src; + header_alignment_sync = primary_header_alignment_src; + } + else { + /* Keep on opposite sides. */ + tool_header_alignment_sync = primary_header_alignment_src; + header_alignment_sync = (tool_header_alignment_sync == RGN_ALIGN_BOTTOM) ? + RGN_ALIGN_TOP : + RGN_ALIGN_BOTTOM; + } + } + else { + /* Apply what we can to regions that exist. */ + if (header_alignment_dst != -1) { + header_alignment_sync = primary_header_alignment_src; + } + if (tool_header_alignment_dst != -1) { + tool_header_alignment_sync = primary_header_alignment_src; + } + } + } + } + + if (footer_alignment_dst != -1) { + if ((header_alignment_dst != -1) && (header_alignment_dst == footer_alignment_dst)) { + /* Apply to both. */ + footer_alignment_sync = primary_header_alignment_src; + } + else { + /* Keep on opposite sides. */ + footer_alignment_sync = (primary_header_alignment_src == RGN_ALIGN_BOTTOM) ? + RGN_ALIGN_TOP : + RGN_ALIGN_BOTTOM; + } + } + + /* Finally apply synchronized flags. */ + if (header_alignment_sync != -1) { + ARegion *region = region_by_type[RGN_TYPE_HEADER]; + if (region != NULL) { + region->alignment = RGN_ALIGN_ENUM_FROM_MASK(header_alignment_sync) | + RGN_ALIGN_FLAG_FROM_MASK(region->alignment); + } + } + + if (tool_header_alignment_sync != -1) { + ARegion *region = region_by_type[RGN_TYPE_TOOL_HEADER]; + if (region != NULL) { + region->alignment = RGN_ALIGN_ENUM_FROM_MASK(tool_header_alignment_sync) | + RGN_ALIGN_FLAG_FROM_MASK(region->alignment); + } + } + + if (footer_alignment_sync != -1) { + ARegion *region = region_by_type[RGN_TYPE_FOOTER]; + if (region != NULL) { + region->alignment = RGN_ALIGN_ENUM_FROM_MASK(footer_alignment_sync) | + RGN_ALIGN_FLAG_FROM_MASK(region->alignment); + } + } +} + +static void region_align_info_to_area( + ScrArea *area, const struct RegionTypeAlignInfo region_align_info_src[RGN_TYPE_LEN]) +{ + ARegion *region_by_type[RGN_TYPE_LEN] = {NULL}; + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + const int index = region->regiontype; + if ((uint)index < RGN_TYPE_LEN) { + region_by_type[index] = region; + } + } + + struct RegionTypeAlignInfo region_align_info_dst; + region_align_info_from_area(area, ®ion_align_info_dst); + + if ((region_by_type[RGN_TYPE_HEADER] != NULL) || + (region_by_type[RGN_TYPE_TOOL_HEADER] != NULL)) { + region_align_info_to_area_for_headers( + region_align_info_src, ®ion_align_info_dst, region_by_type); + } + + /* Note that we could support other region types. */ +} + +/** \} */ + /* *********** Space switching code *********** */ void ED_area_swapspace(bContext *C, ScrArea *sa1, ScrArea *sa2) @@ -2104,9 +2339,13 @@ void ED_area_newspace(bContext *C, ScrArea *area, int type, const bool skip_regi * the space type defaults to in this case instead * (needed for preferences to have space-type on bottom). */ - int header_alignment = ED_area_header_alignment_or_fallback(area, -1); - const bool sync_header_alignment = ((header_alignment != -1) && - ((slold->link_flag & SPACE_FLAG_TYPE_TEMPORARY) == 0)); + + bool sync_header_alignment = false; + struct RegionTypeAlignInfo region_align_info[RGN_TYPE_LEN]; + if ((slold != NULL) && (slold->link_flag & SPACE_FLAG_TYPE_TEMPORARY) == 0) { + region_align_info_from_area(area, region_align_info); + sync_header_alignment = true; + } /* in some cases (opening temp space) we don't want to * call area exit callback, so we temporarily unset it */ @@ -2180,28 +2419,7 @@ void ED_area_newspace(bContext *C, ScrArea *area, int type, const bool skip_regi /* Sync header alignment. */ if (sync_header_alignment) { - /* Spaces with footer. */ - if (st->spaceid == SPACE_TEXT) { - LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { - if (ELEM(region->regiontype, RGN_TYPE_HEADER, RGN_TYPE_TOOL_HEADER)) { - region->alignment = header_alignment; - } - if (region->regiontype == RGN_TYPE_FOOTER) { - int footer_alignment = (header_alignment == RGN_ALIGN_BOTTOM) ? RGN_ALIGN_TOP : - RGN_ALIGN_BOTTOM; - region->alignment = footer_alignment; - break; - } - } - } - else { - LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { - if (ELEM(region->regiontype, RGN_TYPE_HEADER, RGN_TYPE_TOOL_HEADER)) { - region->alignment = header_alignment; - break; - } - } - } + region_align_info_to_area(area, region_align_info); } ED_area_initialize(CTX_wm_manager(C), win, area); diff --git a/source/blender/makesdna/DNA_screen_types.h b/source/blender/makesdna/DNA_screen_types.h index d5b828c898d..60d6f85569c 100644 --- a/source/blender/makesdna/DNA_screen_types.h +++ b/source/blender/makesdna/DNA_screen_types.h @@ -635,7 +635,10 @@ typedef enum eRegionType { RGN_TYPE_EXECUTE = 10, RGN_TYPE_FOOTER = 11, RGN_TYPE_TOOL_HEADER = 12, + +#define RGN_TYPE_LEN (RGN_TYPE_TOOL_HEADER + 1) } eRegionType; + /* use for function args */ #define RGN_TYPE_ANY -1 @@ -661,6 +664,7 @@ enum { /** Mask out flags so we can check the alignment. */ #define RGN_ALIGN_ENUM_FROM_MASK(align) ((align) & ((1 << 4) - 1)) +#define RGN_ALIGN_FLAG_FROM_MASK(align) ((align) & ~((1 << 4) - 1)) /** #ARegion.flag */ enum { -- cgit v1.2.3