Evolvee / EvolvePWPUI-ClassicWOTLK

Evolve PWP UI for ClassicWOTLK --- Optimized only for 1920x1080 17,3 inch Laptop!
https://evolvee.github.io/EvolvePWPUI-ClassicWOTLK/
3 stars 1 forks source link

[Bug] ClassPortraits - fps lag spike when switching weapon + texture blinking/refreshing #51

Closed Evolvee closed 2 years ago

Evolvee commented 2 years ago

https://www.youtube.com/watch?v=ttcNHDRE41Y https://youtu.be/gtLJc9wEuCk

XyzKangUI commented 2 years ago

Your method to use OnUpdate for class portrait is horrible. Use a simple hooksecurefunc that will replace half of your code

hooksecurefunc("UnitFramePortrait_Update",function(self)
    if self.unit == "player" then
        self.portrait:SetTexture("Interface\\Addons\\ClassPortraits\\MYSKIN")
        return
    elseif self.unit == "pet" then
        return
    end

    if self.portrait then
        if UnitIsPlayer(self.unit) then
            local t = CLASS_ICON_TCOORDS[select(2,UnitClass(self.unit))]
            if t then
                self.portrait:SetTexture(iconPath)
                self.portrait:SetTexCoord(unpack(t))
            end
        else
            self.portrait:SetTexCoord(0,1,0,1)
        end
    end
end);
Evolvee commented 2 years ago

I think this method is used in all the classportraits addons out there, but it was causing extreme fps issue on tbc beta - https://github.com/leatrix/leatrix-plus-wrath/issues/7 so I switched to the old 2.4.3 method I was using.

Perhaps blizzard already fixed their shit, will test it.

Evolvee commented 2 years ago

image

they didnt fix shit, unit portraits are refreshed every frame 24/7

Evolvee commented 2 years ago

it doesnt seem to cause as extreme fps issue as it did though, thx for suggestion, i will use it during the prepatch some more and test it against the last version

XyzKangUI commented 2 years ago

You were using OnUpdate, I don't see how that was any better by not having unit portraits being refreshed every frame 24/7. On top of your own OnUpdate there is another OnUpdate being used by Blizzard. Essentially UnitFramePortrait_Update is only called by two events, which is fine. The expensive part is that it is also being used in UnitFrame_Update which is invoked by TargetFrame_OnUpdate when a ToT is shown. With or without this hook you will have that spam you showed, simply because of Blizzard's code. The code itself to change portraits gets the blame because its hooked into the function, even an empty hook would do the same.

Although I've noticed with a different addon that changing texture on an OnUpdate function can drop fps by a lot. So to avoid this you can only add something like if self.unit == "targettarget" or self.unit == "focus-target" then return end. That way your class portrait code will have the least impact on your fps (updates only on portrait changed event) at the cost of not having class portraits on ToT frames. For me the cost of having ToT class portraits is only 1-2 fps, so it is negligible, but I can believe that you may experience way more frame drops like I do with that other addon dropping my fps by 16.

XyzKangUI commented 2 years ago

Actually re-using a part of your old code we can make the most efficient class portrait update hook without changing ToT texture on every frame.

hooksecurefunc("UnitFramePortrait_Update", function(self)
    if self.unit == "player" then
        self.portrait:SetTexture("Interface\\Addons\\ClassPortraits\\MYSKIN")
        return
    elseif self.unit == "pet" then
        return
    end

    if self.portrait and not (self.unit == "targettarget" or self.unit == "focus-target") then
        if UnitIsPlayer(self.unit) then
            local t = CLASS_ICON_TCOORDS[select(2,UnitClass(self.unit))]
            if t then
                self.portrait:SetTexture(iconPath)
                self.portrait:SetTexCoord(unpack(t))
            end
        else
            self.portrait:SetTexCoord(0,1,0,1)
        end
    end

    if UnitExists("targettarget") ~= nil then
        if UnitGUID("targettarget") ~= lastTargetToTGuid then
            lastTargetToTGuid = UnitGUID("targettarget")
            if UnitIsPlayer("targettarget") then
                TargetToTPortrait:SetTexture(iconPath, true)
                local tt=classIcons[(select(2, UnitClass("targettarget")))]
                TargetToTPortrait:SetTexCoord(unpack(tt))
                TargetToTPortrait:Show()
            else
                TargetToTPortrait:Hide()
            end
        end
    else
        TargetToTPortrait:Hide()
        lastTargetToTGuid = nil
    end

    if UnitExists("focus-target") ~= nil then
        if UnitGUID("focus-target") ~= lastFocusToTGuid then
            lastFocusToTGuid = UnitGUID("focus-target")
            if UnitIsPlayer("focus-target") then
                FocusToTPortrait:SetTexture(iconPath, true)
                local tt=classIcons[(select(2, UnitClass("focus-target")))]
                FocusToTPortrait:SetTexCoord(unpack(tt))
                FocusToTPortrait:Show()
            else
                FocusToTPortrait:Hide()
            end
        end
    else
        FocusToTPortrait:Hide()
        lastFocusToTGuid = nil
    end
end)
Evolvee commented 2 years ago

Thanks! ToT is not that important for me, as im playing on a laptop, I value every bit of fps very much, especially due to 120hz monitor any drop below stable 120fps is very noticable.

On wotlk however, I noticed these drops occur even with no addons whatsoever, which wasnt rly happening with TBC that much, hopefully they will fix it (Copium, we all know they wont fix shit)

XyzKangUI commented 2 years ago

I also noticed during wotlk beta that my game would freeze sometimes, for example when i got an achievement. Luckily they fixed that, but I have no high hopes as well and will not even bother with wotlk. Btw im totally implementing that portrait code to my own addon 😄